* Removing items from the BOutlineListView should longer crash the application.

* Also, the selection should be updated correctly when items are removed.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@16652 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/kits/interface/ListView.cpp

@@ -544,7 +544,6 @@ BListItem*
 BListView::RemoveItem(int32 index)
 {
 	BListItem *item = ItemAt(index);
-
 	if (!item)
 		return NULL;
 
@@ -570,7 +569,7 @@ BListView::RemoveItem(int32 index)
 bool
 BListView::RemoveItem(BListItem *item)
 {
-	return RemoveItem(IndexOf(item)) != NULL;
+	return BListView::RemoveItem(IndexOf(item)) != NULL;
 }
 
 // RemoveItems
@@ -585,7 +584,7 @@ BListView::RemoveItems(int32 index, int32 count)
 
 	// TODO: very bad for performance!!
 	while (count--)
-		RemoveItem(index);
+		BListView::RemoveItem(index);
 
 	return true;
 }

src/kits/interface/OutlineListView.cpp

@@ -250,33 +250,14 @@ BOutlineListView::AddList(BList* newItems, int32 fullListIndex)
 bool
 BOutlineListView::RemoveItem(BListItem* item)
 {
-	if (!FullListHasItem(item))
+	return _RemoveItem(item, FullListIndexOf(item)) != NULL;
-		return false;
-
-	fFullList.RemoveItem(item);
-	if (item->fVisible)
-		BListView::RemoveItem(item);
-
-	// TODO: remove children
-
-	return true;
 }
 
 
 BListItem*
-BOutlineListView::RemoveItem(int32 fullListIndex)
+BOutlineListView::RemoveItem(int32 fullIndex)
 {
-	BListItem* item = FullListItemAt(fullListIndex);
+	return _RemoveItem(FullListItemAt(fullIndex), fullIndex);
-	if (item == NULL)
-		return NULL;
-
-	fFullList.RemoveItem(fullListIndex);
-	if (item->fVisible)
-		BListView::RemoveItem(item);
-
-	// TODO: remove children
-
-	return item;
 }
 
 
@@ -401,9 +382,9 @@ BOutlineListView::Expand(BListItem* item)
 	item->fExpanded = true;
 
 	uint32 level = item->fLevel;
-	uint32 fullIndex = FullListIndexOf(item);
+	int32 fullIndex = FullListIndexOf(item);
-	uint32 index = IndexOf(item) + 1;
+	int32 index = IndexOf(item) + 1;
-	uint32 count = FullListCountItems() - fullIndex - 1;
+	int32 count = FullListCountItems() - fullIndex - 1;
 	BListItem** items = (BListItem**)fFullList.Items() + fullIndex + 1;
 
 	BFont font;
@@ -415,6 +396,12 @@ BOutlineListView::Expand(BListItem* item)
 			break;
 
 		if (!item->IsItemVisible()) {
+			// fix selection hints
+			if (index <= fFirstSelected)
+				fFirstSelected++;
+			if (index <= fLastSelected)
+				fLastSelected++;
+
 			fList.AddItem(item, index++);
 			item->Update(this, &font);
 			item->SetItemVisible(true);
@@ -445,11 +432,13 @@ BOutlineListView::Collapse(BListItem* item)
 	item->fExpanded = false;
 
 	uint32 level = item->fLevel;
-	uint32 fullIndex = FullListIndexOf(item);
+	int32 fullIndex = FullListIndexOf(item);
-	uint32 count = FullListCountItems() - fullIndex - 1;
+	int32 index = IndexOf(item);
+	int32 max = FullListCountItems() - fullIndex - 1;
+	int32 count = 0;
 	BListItem** items = (BListItem**)fFullList.Items() + fullIndex + 1;
 
-	while (count-- > 0) {
+	while (max-- > 0) {
 		item = items[0];
 		if (item->fLevel <= level)
 			break;
@@ -457,11 +446,29 @@ BOutlineListView::Collapse(BListItem* item)
 		if (item->IsItemVisible()) {
 			fList.RemoveItem(item);
 			item->SetItemVisible(false);
+			if (item->IsSelected())
+				item->Deselect();
+
+			count++;
 		}
 
 		items++;
 	}
 
+	// fix selection hints
+	// TODO: revise for multi selection lists
+	if (index < fFirstSelected) {
+		if (index + count < fFirstSelected) {
+			fFirstSelected -= count;
+			fLastSelected -= count;
+		} else {
+			// select top item
+			//fFirstSelected = fLastSelected = index;
+			//item->Select();
+			Select(index);
+		}
+	}
+
 	_FixupScrollBar();
 	Invalidate();
 }
@@ -850,10 +857,45 @@ BOutlineListView::DrawItem(BListItem* item, BRect itemRect, bool complete)
 }
 
 
+/*!
+	\brief Removes a single item from the list and all of its children.
+
+	Unlike the BeOS version, this one will actually delete the children, too,
+	as there should be no reference left to them. This may cause problems for
+	applications that actually take the misbehaviour of the Be classes into
+	account.
+*/
 BListItem *
-BOutlineListView::RemoveCommon(int32 fullListIndex)
+BOutlineListView::_RemoveItem(BListItem* item, int32 fullIndex)
 {
-	return NULL;
+	if (item == NULL || fullIndex < 0 || fullIndex >= FullListCountItems())
+		return NULL;
+
+	if (item->IsItemVisible()) {
+		// remove children, too
+		uint32 level = item->OutlineLevel();
+		int32 max = FullListCountItems() - fullIndex - 1;
+		BListItem** items = (BListItem**)fFullList.Items() + fullIndex + 1;
+
+		while (max-- > 0) {
+			BListItem* subItem = items[0];
+			if (subItem->fLevel <= level)
+				break;
+
+			if (subItem->IsItemVisible())
+				BListView::RemoveItem(subItem);
+
+			fFullList.RemoveItem(fullIndex + 1);
+
+			// TODO: this might be problematic, see comment above
+			delete subItem;
+		}
+
+		BListView::RemoveItem(item);
+	}
+
+	fFullList.RemoveItem(fullIndex);
+	return item;
 }