BOutlineListView: fix ItemUnderAt and accept NULL as superitem

ItemUnderAt was returning items that were not under the superitem (this
was fixed for EachItemUnder in hrev52210).
Make a NULL superitem mean the parent of the topmost items. Despite not
being explicitly documented in the BeBook, that's how BeOS works, the
MenuWorld test app uses it and, well, it's handy.

Change-Id: I2551e8ce874a6238c5e5fb1eb742e68e62d3928a
Reviewed-on: https://review.haiku-os.org/c/haiku/+/7359
Reviewed-by: Adrien Destugues <[email protected]>

docs/user/interface/OutlineListView.dox

@@ -580,7 +580,8 @@
 		bool oneLevelOnly) const
 	\brief Returns the number of items under \a superItem.
 
-	\param superItem The base item.
+	\param superItem The base item, or NULL to use the virtual super item
+	       of level 0 items.
 	\param oneLevelOnly if \c true, only items located one level under
 	       superItem are considered.
 
@@ -596,7 +597,8 @@
 		void* arg)
 	\brief Calls \a eachFunc for each item under \a superItem.
 
-	\param superItem The base item.
+	\param superItem The base item, or NULL to use the virtual super item
+	       of level 0 items.
 	\param oneLevelOnly if \c true, only items located one level under
 	       superItem are considered.
 	\param eachFunc The function to call on each item.
@@ -611,7 +613,8 @@
 		bool oneLevelOnly, int32 index) const
 	\brief Returns a pointer to the item at \a index under \a superItem.
 
-	\param superItem The base item.
+	\param superItem The base item, or NULL to use the virtual super item
+	       of level 0 items.
 	\param oneLevelOnly if \c true, only items located one level under
 	       superItem are considered.
 	\param index The index of the item to get.

headers/os/interface/OutlineListView.h

@@ -139,6 +139,8 @@ private:
 									int32 level, int32* _superIndex = NULL);
 			int32				_FindPreviousVisibleIndex(int32 fullListIndex);
 
+			status_t			_ItemsUnderSetup(BListItem* superItem, int32& startIndex,
+									uint32& baseLevel) const;
 private:
 			BList				fFullList;
 

src/kits/interface/OutlineListView.cpp

@@ -697,23 +697,21 @@ BOutlineListView::SortItemsUnder(BListItem* superItem, bool oneLevelOnly,
 int32
 BOutlineListView::CountItemsUnder(BListItem* superItem, bool oneLevelOnly) const
 {
-	int32 i = FullListIndexOf(superItem);
+	int32 i = 0;
-	if (i == -1)
+	uint32 baseLevel = 0;
+	if (_ItemsUnderSetup(superItem, i, baseLevel) != B_OK)
 		return 0;
 
-	++i;
 	int32 count = 0;
-	uint32 baseLevel = superItem->OutlineLevel();
-
 	for (; i < FullListCountItems(); i++) {
 		BListItem* item = FullListItemAt(i);
 
 		// If we jump out of the subtree, return count
-		if (item->fLevel <= baseLevel)
+		if (item->fLevel < baseLevel)
 			return count;
 
 		// If the level matches, increase count
-		if (!oneLevelOnly || item->fLevel == baseLevel + 1)
+		if (!oneLevelOnly || item->fLevel == baseLevel)
 			count++;
 	}
 
@@ -725,20 +723,20 @@ BListItem*
 BOutlineListView::EachItemUnder(BListItem* superItem, bool oneLevelOnly,
 	BListItem* (*eachFunc)(BListItem* item, void* arg), void* arg)
 {
-	int32 i = FullListIndexOf(superItem);
+	int32 i = 0;
-	if (i == -1)
+	uint32 baseLevel = 0;
+	if (_ItemsUnderSetup(superItem, i, baseLevel) != B_OK)
 		return NULL;
 
-	i++; // skip the superitem
 	while (i < FullListCountItems()) {
 		BListItem* item = FullListItemAt(i);
 
 		// If we jump out of the subtree, return NULL
-		if (item->fLevel <= superItem->OutlineLevel())
+		if (item->fLevel < baseLevel)
 			return NULL;
 
 		// If the level matches, check the index
-		if (!oneLevelOnly || item->fLevel == superItem->OutlineLevel() + 1) {
+		if (!oneLevelOnly || item->fLevel == baseLevel) {
 			item = eachFunc(item, arg);
 			if (item != NULL)
 				return item;
@@ -755,19 +753,20 @@ BListItem*
 BOutlineListView::ItemUnderAt(BListItem* superItem, bool oneLevelOnly,
 	int32 index) const
 {
-	int32 i = FullListIndexOf(superItem);
+	int32 i = 0;
-	if (i == -1)
+	uint32 baseLevel = 0;
+	if (_ItemsUnderSetup(superItem, i, baseLevel) != B_OK)
 		return NULL;
 
 	while (i < FullListCountItems()) {
 		BListItem* item = FullListItemAt(i);
 
 		// If we jump out of the subtree, return NULL
-		if (item->fLevel < superItem->OutlineLevel())
+		if (item->fLevel < baseLevel)
 			return NULL;
 
 		// If the level matches, check the index
-		if (!oneLevelOnly || item->fLevel == superItem->OutlineLevel() + 1) {
+		if (!oneLevelOnly || item->fLevel == baseLevel) {
 			if (index == 0)
 				return item;
 
@@ -1209,3 +1208,19 @@ BOutlineListView::_FindPreviousVisibleIndex(int32 fullListIndex)
 
 	return -1;
 }
+
+
+status_t
+BOutlineListView::_ItemsUnderSetup(BListItem* superItem, int32& startIndex, uint32& baseLevel) const
+{
+	if (superItem != NULL) {
+		startIndex = FullListIndexOf(superItem) + 1;
+		if (startIndex == 0)
+			return B_ENTRY_NOT_FOUND;
+		baseLevel = superItem->OutlineLevel() + 1;
+	} else {
+		startIndex = 0;
+		baseLevel = 0;
+	}
+	return B_OK;
+}

src/tests/kits/interface/OutlineListViewTest.cpp

@@ -19,17 +19,54 @@ int gIndex = 0;
 int gCount = 0;
 
 
-BListItem* eachitemunder(BListItem* item, void* arg) {
+template<>
+struct CppUnit::assertion_traits<BListItem*>
+{
+	static bool equal(const BListItem* x, const BListItem* y) {
+		return x == y;
+	}
+
+	static string toString(const BListItem* x) {
+		if (x == NULL)
+			return "(null)";
+		return ((BStringItem*)x)->Text();
+	}
+};
+
+
+BListItem*
+CheckExpected(BListItem* item, void* arg)
+{
 	BStringItem* str = (BStringItem*)item;
-	fprintf(stderr, "Item @%d: %s\n", gIndex, str->Text());
+	fprintf(stderr, "Item @%d: %s\n", gIndex, str == NULL ? "(null)" : str->Text());
 
 	CHK(gIndex < gCount);
-	CPPUNIT_ASSERT_EQUAL(item, gExpected[gIndex]);
+	CPPUNIT_ASSERT_EQUAL(gExpected[gIndex], item);
 	gIndex++;
 	return NULL;
 }
 
 
+BListItem*
+FillExpected(BListItem* item, void* arg)
+{
+	gExpected[gCount] = item;
+	gCount++;
+	return NULL;
+}
+
+
+void
+CheckItemsUnder(BOutlineListView* view, BListItem* superitem, bool oneLevelOnly)
+{
+	for (int i = 0; i < gCount; i++)
+		CPPUNIT_ASSERT_EQUAL(gExpected[i], view->ItemUnderAt(superitem, oneLevelOnly, i));
+
+	// Check that we don't get more items
+	CPPUNIT_ASSERT_EQUAL((BListItem*)NULL, view->ItemUnderAt(superitem, oneLevelOnly, gCount));
+}
+
+
 class OutlineListViewTest: public TestCase
 {
 	public:
@@ -37,18 +74,210 @@ class OutlineListViewTest: public TestCase
 		OutlineListViewTest(std::string name) : TestCase(name) {}
 
 		void EachItemUnder();
+		void AddUnder();
+		void ItemUnderAt();
 
 		static Test* Suite();
+
+	private:
+		static BOutlineListView* _SetupTest(const char* name);
 };
 
 
-void OutlineListViewTest::EachItemUnder() {
+void
-	BApplication app(
+OutlineListViewTest::EachItemUnder()
-		"application/x-vnd.OutlineListView_EachItemUnder.test");
+{
-	BWindow* window = new BWindow(BRect(50,50,550,550),
+	BOutlineListView* view = _SetupTest("OutlineListView_EachItemUnder");
-		"OutlineListView_EachItemUnder", B_TITLED_WINDOW,
+
-		B_QUIT_ON_WINDOW_CLOSE, 0);
+	// First test is easy
-	BOutlineListView* view = new BOutlineListView(BRect(5,5,495,495), "View",
+	gExpected[0] = view->FullListItemAt(6);
+	gExpected[1] = view->FullListItemAt(8);
+	gExpected[2] = view->FullListItemAt(9);
+	gCount = 3;
+	gIndex = 0;
+
+	fprintf(stderr, "Easy test\n");
+	view->EachItemUnder(view->FullListItemAt(5), true, CheckExpected, NULL);
+	CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), true));
+
+	// Check that collapsing an item does not change the outcome
+	gIndex = 0;
+	view->Collapse(view->FullListItemAt(0));
+
+	fprintf(stderr, "One collapsed\n");
+	view->EachItemUnder(view->FullListItemAt(5), true, CheckExpected, NULL);
+
+	gIndex = 0;
+	view->Collapse(view->FullListItemAt(5));
+
+	fprintf(stderr, "Two collapsed\n");
+	view->EachItemUnder(view->FullListItemAt(5), true, CheckExpected, NULL);
+	CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), true));
+
+	// Also check deeper levels
+	gExpected[1] = view->FullListItemAt(7);
+	gExpected[2] = view->FullListItemAt(8);
+	gExpected[3] = view->FullListItemAt(9);
+	gCount = 4;
+	gIndex = 0;
+
+	fprintf(stderr, "All levels\n");
+	view->EachItemUnder(view->FullListItemAt(5), false, CheckExpected, NULL);
+	CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), false));
+
+	view->Expand(view->FullListItemAt(5));
+	view->Collapse(view->FullListItemAt(6));
+	gIndex = 0;
+
+	fprintf(stderr, "All levels with a collapsed sublevel\n");
+	view->EachItemUnder(view->FullListItemAt(5), false, CheckExpected, NULL);
+	CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), false));
+
+	// NULL is the parent of level 0 items
+	gExpected[0] = view->FullListItemAt(0);
+	gExpected[1] = view->FullListItemAt(5);
+	gExpected[2] = view->FullListItemAt(10);
+	gCount = 3;
+	gIndex = 0;
+
+	fprintf(stderr, "Level 0\n");
+	view->EachItemUnder(NULL, true, CheckExpected, NULL);
+	CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(NULL, true));
+
+	// No visits when the item is not in the list
+	BListItem* notfound = new BStringItem("Not found");
+	gCount = 0;
+	gIndex = 0;
+
+	fprintf(stderr, "Item not in the list\n");
+	view->EachItemUnder(notfound, true, CheckExpected, NULL);
+	CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(notfound, true));
+	view->EachItemUnder(notfound, false, CheckExpected, NULL);
+	CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(notfound, false));
+
+	// Don't actually run anything
+	delete view->Window();
+}
+
+
+void
+OutlineListViewTest::AddUnder()
+{
+	BOutlineListView* view = _SetupTest("OutlineListView_AddUnder");
+
+	BListItem* one = view->FullListItemAt(0);
+	BListItem* oneA = view->FullListItemAt(1);
+	BListItem* oneA0 = new BStringItem("One-A-0");
+	BListItem* oneA1 = view->FullListItemAt(2);
+
+	int32 count = view->FullListCountItems();
+
+	BListItem* last = view->FullListItemAt(count - 1);
+	BListItem* newLast = new BStringItem("NewLast");
+
+	view->AddUnder(newLast, NULL);
+	view->AddUnder(oneA0, oneA);
+
+	fprintf(stderr, "Count\n");
+	CPPUNIT_ASSERT_EQUAL(count + 2, view->FullListCountItems());
+
+	fprintf(stderr, "Insertion order\n");
+	CPPUNIT_ASSERT_EQUAL(one, view->FullListItemAt(0));
+	CPPUNIT_ASSERT_EQUAL(oneA, view->FullListItemAt(1));
+	CPPUNIT_ASSERT_EQUAL(oneA0, view->FullListItemAt(2));
+	CPPUNIT_ASSERT_EQUAL(oneA1, view->FullListItemAt(3));
+	CPPUNIT_ASSERT_EQUAL(last, view->FullListItemAt(count));
+	CPPUNIT_ASSERT_EQUAL(newLast, view->FullListItemAt(count + 1));
+
+	fprintf(stderr, "Levels\n");
+	CPPUNIT_ASSERT_EQUAL(0, one->OutlineLevel());
+	CPPUNIT_ASSERT_EQUAL(1, oneA->OutlineLevel());
+	CPPUNIT_ASSERT_EQUAL(2, oneA0->OutlineLevel());
+	CPPUNIT_ASSERT_EQUAL(2, oneA1->OutlineLevel());
+	CPPUNIT_ASSERT_EQUAL(0, newLast->OutlineLevel());
+
+	// Don't actually run anything
+	delete view->Window();
+}
+
+
+void
+OutlineListViewTest::ItemUnderAt()
+{
+	BOutlineListView* view = _SetupTest("OutlineListView_ItemUnderAt");
+
+	// EachItemUnder has already been checked, we can use it to know what to expect
+	gCount = 0;
+	view->EachItemUnder(view->FullListItemAt(5), true, FillExpected, NULL);
+
+	fprintf(stderr, "Easy test\n");
+	CheckItemsUnder(view, view->FullListItemAt(5), true);
+
+	// Check that collapsing an item does not change the outcome
+	view->Collapse(view->FullListItemAt(0));
+
+	fprintf(stderr, "One collapsed\n");
+	CheckItemsUnder(view, view->FullListItemAt(5), true);
+
+	view->Collapse(view->FullListItemAt(5));
+
+	fprintf(stderr, "Two collapsed\n");
+	CheckItemsUnder(view, view->FullListItemAt(5), true);
+
+	// Also check deeper levels
+	gCount = 0;
+	view->EachItemUnder(view->FullListItemAt(5), false, FillExpected, NULL);
+
+	fprintf(stderr, "All levels\n");
+	CheckItemsUnder(view, view->FullListItemAt(5), false);
+
+	view->Expand(view->FullListItemAt(5));
+	view->Collapse(view->FullListItemAt(6));
+
+	fprintf(stderr, "All levels with a collapsed sublevel\n");
+	CheckItemsUnder(view, view->FullListItemAt(5), false);
+
+	// NULL is the parent of level 0 items
+	gCount = 0;
+	view->EachItemUnder(NULL, true, FillExpected, NULL);
+
+	fprintf(stderr, "Level 0\n");
+	CheckItemsUnder(view, NULL, true);
+
+	// Get NULL when the item is not in the list
+	BListItem* notfound = new BStringItem("Not found");
+	fprintf(stderr, "Item not in the list\n");
+	CPPUNIT_ASSERT_EQUAL((BListItem*)NULL, view->ItemUnderAt(notfound, true, 0));
+	CPPUNIT_ASSERT_EQUAL((BListItem*)NULL, view->ItemUnderAt(notfound, false, 0));
+
+	// Don't actually run anything
+	delete view->Window();
+}
+
+
+Test*
+OutlineListViewTest::Suite()
+{
+	TestSuite* SuiteOfTests = new TestSuite;
+
+	ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest, EachItemUnder);
+	ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest, AddUnder);
+	ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest, ItemUnderAt);
+
+	return SuiteOfTests;
+}
+
+
+BOutlineListView*
+OutlineListViewTest::_SetupTest(const char* name)
+{
+	if (be_app == NULL)
+		new BApplication("application/x-vnd.OutlineListView.test");
+
+	BWindow* window = new BWindow(BRect(50, 50, 550, 550), name,
+		B_TITLED_WINDOW, B_QUIT_ON_WINDOW_CLOSE, 0);
+
+	BOutlineListView* view = new BOutlineListView(BRect(5, 5, 495, 495), "View",
 		B_MULTIPLE_SELECTION_LIST, B_FOLLOW_ALL);
 	window->AddChild(view);
 
@@ -70,42 +299,7 @@ void OutlineListViewTest::EachItemUnder() {
 	view->AddItem(new BStringItem("Three-B", 1));
 	view->AddItem(new BStringItem("Three-C", 1));
 
-	// First test is easy
+	return view;
-	gExpected[0] = view->FullListItemAt(6);
-	gExpected[1] = view->FullListItemAt(8);
-	gExpected[2] = view->FullListItemAt(9);
-	gCount = 3;
-	gIndex = 0;
-
-	fprintf(stderr, "Easy test\n");
-	view->EachItemUnder(view->FullListItemAt(5), true, eachitemunder, NULL);
-
-	// Check that collapsing an item does not change the outcome
-	gIndex = 0;
-	view->Collapse(view->FullListItemAt(0));
-
-	fprintf(stderr, "One collapsed\n");
-	view->EachItemUnder(view->FullListItemAt(5), true, eachitemunder, NULL);
-
-	gIndex = 0;
-	view->Collapse(view->FullListItemAt(5));
-
-	fprintf(stderr, "Two collapsed\n");
-	view->EachItemUnder(view->FullListItemAt(5), true, eachitemunder, NULL);
-
-	// Don't actually run anything
-	delete window;
-}
-
-
-Test* OutlineListViewTest::Suite()
-{
-	TestSuite* SuiteOfTests = new TestSuite;
-
-	ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest,
-		EachItemUnder);
-
-	return SuiteOfTests;
 }