Fix #5788 by overhauling the Locale preflet a bit:

* InitiateDrag() now makes sure that either a number of top-level items
  or a group of subitems sharing the parent is being dragged - anything
  else doesn't really make sense and results in mayhem and/or crashes
* LocaleWindow now tries harder to keep the language listview sorted
* when moving around items between the two listviews, they are no longer
  copied (and partly leaked), but the same items are now just moved over
* cleanup here and there
This is still somewhat of a mess - I recommend splitting the two listviews at least (as they actually behave differently) - maybe it would be better to pick the available languages from a popup-menu instead of keeping them in a listview?

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

src/preferences/locale/LanguageListView.cpp

@@ -5,6 +5,7 @@
  * Authors:
  *		Stephan Aßmus <[email protected]>
  *		Adrien Destugues <[email protected]>
+ *		Oliver Tappe <[email protected]>
 */
 #include "LanguageListView.h"
 
@@ -39,15 +40,6 @@ LanguageListItem::LanguageListItem(const char* text, const char* code)
 }
 
 
-LanguageListItem::LanguageListItem(const LanguageListItem& other)
-	:
-	BStringItem(other.Text()),
-	fLanguageCode(other.fLanguageCode),
-	fIcon(other.fIcon ? new(std::nothrow) BBitmap(other.fIcon) : NULL)
-{
-}
-
-
 LanguageListItem::~LanguageListItem()
 {
 	delete fIcon;
@@ -131,31 +123,40 @@ LanguageListView::MoveItems(BList& items, int32 index)
 	// or sublevels within the same top level
 
 	DeselectAll();
-	// we remove the items while we look at them, the insertion index is
-	// decreaded when the items index is lower, so that we insert at the right
-	// spot after removal
-	BList removedItems;
+
+	// collect all items that must be moved (adding subitems as necessary)
 	int32 count = items.CountItems();
-	// We loop in the reverse way so we can remove childs before their parents
-	for (int32 i = count - 1; i >= 0; i--) {
+	BList itemsToBeMoved;
+	for (int32 i = 0; i < count; i++) {
 		BListItem* item = (BListItem*)items.ItemAt(i);
-		int32 removeIndex = IndexOf(item);
-		// TODO : remove all childs before removing the item itself, or else
-		// they will be lost forever
-		if (RemoveItem(item) && removedItems.AddItem((void*)item, 0)) {
+		itemsToBeMoved.AddItem(item);
+		if (item->OutlineLevel() == 0) {
+			// add all subitems, as they need to be moved, too
+			int32 subItemCount = CountItemsUnder(item, true);
+			for (int subIndex = 0; subIndex < subItemCount ; subIndex++)
+				itemsToBeMoved.AddItem(ItemUnderAt(item, true, subIndex));
+		}
+	}
+
+	// now remove all the items backwards (in order to remove children before
+	// their parent), decreasing target index if we are removing items before
+	// it
+	count = itemsToBeMoved.CountItems();
+	for (int32 i = count - 1; i >= 0; i--) {
+		BListItem* item = (BListItem*)itemsToBeMoved.ItemAt(i);
+		int32 removeIndex = FullListIndexOf(item);
+		if (RemoveItem(item)) {
 			if (removeIndex < index)
 				index--;
 		}
-		// else ??? -> blow up
 	}
-	for (int32 i = 0; BListItem* item = (BListItem*)removedItems.ItemAt(i);
-		i++) {
-		if (AddItem(item, index)) {
-			// after we're done, the newly inserted items will be selected
-			Select(index, true);
-			// next items will be inserted after this one
+
+	// finally add all the items at the given index
+	for (int32 i = 0; i < count; i++) {
+		BListItem* item = (BListItem*)itemsToBeMoved.ItemAt(i);
+		if (AddItem(item, index))
 			index++;
-		} else
+		else
 			delete item;
 	}
 }
@@ -168,12 +169,12 @@ void LanguageListView::MessageReceived (BMessage* message)
 		LanguageListView* list = NULL;
 		if (message->FindPointer("list", (void**)&list) == B_OK) {
 			// It comes from a list
+			int32 count = CountItems();
+			if (fDropIndex < 0 || fDropIndex > count)
+				fDropIndex = count;
+
 			if (list == this) {
 				// It comes from ourselves : move the item around in the list
-				int32 count = CountItems();
-				if (fDropIndex < 0 || fDropIndex > count)
-					fDropIndex = count;
-
 				BList items;
 				int32 index;
 				for (int32 i = 0;
@@ -209,9 +210,6 @@ void LanguageListView::MessageReceived (BMessage* message)
 				fDropIndex = -1;
 			} else {
 				// It comes from another list : move it here
-				int32 count = CountItems();
-				if (fDropIndex < 0 || fDropIndex > count)
-					fDropIndex = count;
 
 				// ensure we always drop things at top-level and not
 				// in the middle of another outline
@@ -223,14 +221,17 @@ void LanguageListView::MessageReceived (BMessage* message)
 
 				// Item is now a top level one - we must insert just below its
 				// last child
-				fDropIndex += CountItemsUnder(FullListItemAt(fDropIndex),false)
+				fDropIndex += CountItemsUnder(FullListItemAt(fDropIndex), true)
 					+ 1;
 
-				int32 index;
-				for (int32 i = 0;
-					message->FindInt32("index", i, &index) == B_OK; i++) {
-					MoveItemFrom(list, index, fDropIndex);
-					fDropIndex++;
+				int32 indexCount;
+				type_code dummy;
+				if (message->GetInfo("index", &dummy, &indexCount) == B_OK) {
+					for (int32 i = indexCount - 1; i >= 0; i--) {
+						int32 index;
+						if (message->FindInt32("index", i, &index) == B_OK)
+							MoveItemFrom(list, index, fDropIndex);
+					}
 				}
 
 				fDropIndex = -1;
@@ -247,27 +248,36 @@ LanguageListView::MoveItemFrom(BOutlineListView* origin, int32 index,
 	int32 dropSpot)
 {
 	// Check that the node we are going to move is a top-level one.
-	// If not, we want his parent instead
+	// If not, we want its parent instead
 	LanguageListItem* itemToMove = static_cast<LanguageListItem*>(
 		origin->Superitem(origin->FullListItemAt(index)));
 	if (itemToMove == NULL) {
-		itemToMove = static_cast<LanguageListItem*>(origin->FullListItemAt(
-				index));
+		itemToMove = static_cast<LanguageListItem*>(
+			origin->FullListItemAt(index));
+		if (itemToMove == NULL)
+			return;
 	} else
 		index = origin->FullListIndexOf(itemToMove);
 
-	int itemCount = origin->CountItemsUnder(itemToMove, true);
-	LanguageListItem* newItem = new LanguageListItem(*itemToMove);
-	this->AddItem(newItem, dropSpot);
-	newItem->SetExpanded(itemToMove->IsExpanded());
-
-	for (int i = 0; i < itemCount ; i++) {
-		LanguageListItem* subItem = static_cast<LanguageListItem*>(
-			origin->ItemUnderAt(itemToMove, true, i));
-		this->AddUnder(new LanguageListItem(*subItem), newItem);
+	// collect all items that must be moved (adding subitems as necessary)
+	BList itemsToBeMoved;
+	itemsToBeMoved.AddItem(itemToMove);
+	// add all subitems, as they need to be moved, too
+	int32 subItemCount = origin->CountItemsUnder(itemToMove, true);
+	for (int32 subIndex = 0; subIndex < subItemCount; subIndex++) {
+		itemsToBeMoved.AddItem(origin->ItemUnderAt(itemToMove, true,
+			subIndex));
 	}
-	origin->RemoveItem(index);
-		// This will also remove the children
+
+	// now remove all items from origin in reverse order (to remove the children
+	// before the parent) ...
+	// TODO: using RemoveItem() on the parent will delete the subitems, which
+	//       may be a bug, actually.
+	int32 itemCount = itemsToBeMoved.CountItems();
+	for (int32 i = itemCount - 1; i >= 0; i--)
+		origin->RemoveItem(index + i);
+	// ... and add all the items to this list
+	AddList(&itemsToBeMoved, dropSpot);
 }
 
 
@@ -277,7 +287,7 @@ LanguageListView::InitiateDrag(BPoint point, int32 index, bool)
 	bool success = false;
 	BListItem* item = FullListItemAt(CurrentSelection(0));
 	if (!item) {
-		// workarround a timing problem
+		// workaround for a timing problem
 		Select(index);
 		item = FullListItemAt(index);
 	}
@@ -285,18 +295,46 @@ LanguageListView::InitiateDrag(BPoint point, int32 index, bool)
 		// create drag message
 		BMessage msg('DRAG');
 		msg.AddPointer("list", (void*)(this));
+		// first selection round, consider only superitems
 		int32 index;
-		for (int32 i = 0; (index = FullListCurrentSelection(i)) >= 0; i++)
-			msg.AddInt32("index", index);
+		for (int32 i = 0; (index = FullListCurrentSelection(i)) >= 0; i++) {
+			BListItem* item = FullListItemAt(index);
+			if (item == NULL)
+				return false;
+			if (item->OutlineLevel() == 0)
+				msg.AddInt32("index", index);
+		}
+		if (!msg.HasInt32("index")) {
+			// second selection round, consider only subitems of the same
+			// (i.e. the first) parent
+			BListItem* seenSuperItem = NULL;
+			for (int32 i = 0; (index = FullListCurrentSelection(i)) >= 0; i++) {
+				BListItem* item = FullListItemAt(index);
+				if (item == NULL)
+					return false;
+				if (item->OutlineLevel() != 0) {
+					BListItem* superItem = Superitem(item);
+					if (seenSuperItem == NULL)
+						seenSuperItem = superItem;
+					if (superItem == seenSuperItem)
+						msg.AddInt32("index", index);
+					else
+						break;
+				}
+			}
+		}
+
 		// figure out drag rect
 		float width = Bounds().Width();
 		BRect dragRect(0.0, 0.0, width, -1.0);
 		// figure out, how many items fit into our bitmap
-		int32 numItems;
 		bool fade = false;
+		int32 numItems;
+		int32 currIndex;
+		BListItem* item;
 		for (numItems = 0;
-			BListItem* item = FullListItemAt(CurrentSelection(numItems));
-			numItems++) {
+			msg.FindInt32("index", numItems, &currIndex) == B_OK
+				&& (item = FullListItemAt(currIndex)) != NULL; numItems++) {
 			dragRect.bottom += ceilf(item->Height()) + 1.0;
 			if (dragRect.Height() > MAX_DRAG_HEIGHT) {
 				fade = true;
@@ -315,7 +353,7 @@ LanguageListView::InitiateDrag(BPoint point, int32 index, bool)
 			itemBounds.bottom = 0.0;
 			// let all selected items, that fit into our drag_bitmap, draw
 			for (int32 i = 0; i < numItems; i++) {
-				int32 index = FullListCurrentSelection(i);
+				int32 index = msg.FindInt32("index", i);
 				LanguageListItem* item
 					= static_cast<LanguageListItem*>(FullListItemAt(index));
 				itemBounds.bottom = itemBounds.top + ceilf(item->Height());

src/preferences/locale/LanguageListView.h

@@ -5,6 +5,7 @@
  * Authors:
  *		Stephan Aßmus <[email protected]>
  *		Adrien Destugues <[email protected]>
+ *		Oliver Tappe <[email protected]>
 */
 #ifndef __LANGUAGE_LIST_VIEW_H
 #define __LANGUAGE_LIST_VIEW_H
@@ -19,7 +20,6 @@ class LanguageListItem : public BStringItem {
 public:
 							LanguageListItem(const char* text,
 								const char* code);
-							LanguageListItem(const LanguageListItem& other);
 	virtual					~LanguageListItem();
 
 			const BString&	LanguageCode() { return fLanguageCode; }

src/preferences/locale/Locale.cpp

@@ -109,10 +109,8 @@ void
 Settings::UpdateFrom(BMessage* message)
 {
 	BPoint point;
-	if (message->FindPoint("window_location", &point) == B_OK) {
-		fMessage.RemoveName("window_location");
-		fMessage.AddPoint("window_location", point);
-	}
+	if (message->FindPoint("window_location", &point) == B_OK)
+		fMessage.ReplacePoint("window_location", point);
 
 	BString langName;
 	// We make sure there is at least one string before erasing the previous
@@ -127,10 +125,9 @@ Settings::UpdateFrom(BMessage* message)
 		}
 	}
 
-	if (message->FindString("country",&langName) == B_OK)
+	if (message->FindString("country", &langName) == B_OK)
 		fMessage.ReplaceString("country", langName);
 
-
 	fUpdated = true;
 }
 

src/preferences/locale/LocaleWindow.cpp

@@ -43,6 +43,15 @@ compare_list_items(const void* _a, const void* _b)
 }
 
 
+static int
+compare_typed_list_items(const BListItem* _a, const BListItem* _b)
+{
+	LanguageListItem* a = *(LanguageListItem**)_a;
+	LanguageListItem* b = *(LanguageListItem**)_b;
+	return strcasecmp(a->Text(), b->Text());
+}
+
+
 LocaleWindow::LocaleWindow()
 	:
 	BWindow(BRect(0, 0, 0, 0), "Locale", B_TITLED_WINDOW, B_NOT_RESIZABLE
@@ -84,7 +93,7 @@ LocaleWindow::LocaleWindow()
 			for (int i = 0; installedLanguages.FindString("langs",
 					i, &currentLanguageCode) == B_OK; i++) {
 
-				// Now get an human-readable, loacalized name for each language
+				// Now get an human-readable, localized name for each language
 				// TODO: sort them using collators.
 				BLanguage* currentLanguage;
 				be_locale_roster->GetLanguage(&currentLanguage,
@@ -101,11 +110,17 @@ LocaleWindow::LocaleWindow()
 					// This is a language without country, add it at top-level
 					fLanguageListView->AddItem(si);
 					si->SetExpanded(false);
+					if (lastAddedLanguage != NULL) {
+						fLanguageListView->SortItemsUnder(lastAddedLanguage,
+							true, compare_typed_list_items);
+					}
 					lastAddedLanguage = si;
 				}
 
 				delete currentLanguage;
 			}
+			fLanguageListView->SortItemsUnder(lastAddedLanguage, true,
+				compare_typed_list_items);
 
 			fLanguageListView->SortItems(compare_list_items);
 				// see previous comment on sort using collators
@@ -133,9 +148,8 @@ LocaleWindow::LocaleWindow()
 		BMessage msg;
 		be_locale_roster->GetPreferredLanguages(&msg);
 		BString langCode;
-		for (int index = 0; msg.FindString("language", index, &langCode)
-					== B_OK;
-				index++) {
+		for (int index = 0;
+			msg.FindString("language", index, &langCode) == B_OK; index++) {
 			for (int listPos = 0; LanguageListItem* lli
 					= static_cast<LanguageListItem*>
 						(fLanguageListView->FullListItemAt(listPos));
@@ -144,8 +158,9 @@ LocaleWindow::LocaleWindow()
 					// We found the item we were looking for, now move it to
 					// the other list along with all its children
 					static_cast<LanguageListView*>(fPreferredListView)
-						-> MoveItemFrom(fLanguageListView, fLanguageListView
-							-> FullListIndexOf(lli));
+						->MoveItemFrom(fLanguageListView,
+							fLanguageListView->FullListIndexOf(lli),
+							fLanguageListView->CountItems());
 				}
 			}
 		}
@@ -247,22 +262,21 @@ LocaleWindow::MessageReceived(BMessage* message)
 
 		case kMsgPrefLanguagesChanged:
 		{
-				BMessage update(kMsgSettingsChanged);
-				int index = 0;
-				while (index < fPreferredListView->FullListCountItems()) {
-					// only include subitems : we can guess the superitem
-					// from them anyway
-					if (fPreferredListView->Superitem(fPreferredListView->
-								FullListItemAt(index))
-							!= NULL) {
-						update.AddString("language",
-							static_cast<LanguageListItem*>
-								(fPreferredListView->FullListItemAt(index))
-							-> LanguageCode());
-					}
-					index++;
+			BMessage update(kMsgSettingsChanged);
+			int index = 0;
+			while (index < fPreferredListView->FullListCountItems()) {
+				// only include subitems : we can guess the superitem
+				// from them anyway
+				if (fPreferredListView->Superitem(
+						fPreferredListView->FullListItemAt(index)) != NULL) {
+					update.AddString("language", static_cast<LanguageListItem*>(
+							fPreferredListView->FullListItemAt(index))
+						->LanguageCode());
 				}
-				be_app_messenger.SendMessage(&update);
+				index++;
+			}
+			fLanguageListView->SortItems(compare_list_items);
+			be_app_messenger.SendMessage(&update);
 			break;
 		}
 
@@ -293,8 +307,7 @@ LocaleWindow::MessageReceived(BMessage* message)
 					= static_cast<LanguageListItem*>
 						(fLanguageListView->RemoveItem(index));
 				fPreferredListView->AddItem(listItem);
-				fPreferredListView
-					->Invoke(fMsgPrefLanguagesChanged);
+				fPreferredListView->Invoke(fMsgPrefLanguagesChanged);
 			}
 			break;
 		}
@@ -312,8 +325,7 @@ LocaleWindow::MessageReceived(BMessage* message)
 				fLanguageListView->AddItem(listItem);
 				fLanguageListView->SortItems(compare_list_items);
 					// see previous comment on sort using collators
-				fPreferredListView
-					->Invoke(fMsgPrefLanguagesChanged);
+				fPreferredListView->Invoke(fMsgPrefLanguagesChanged);
 			}
 			break;
 		}