This a)makes sure the bookmarks are visible in the event Camino was launched via an apple-event b)pops open the search result window when you do a search, in the event it was hidden c)preserves the order of bookmarks dropped in a mass drag & drop (current behavior reverses their order in a folder) d)fixes some button states appropriately depending on what folder is selected e)gets rid of a useless method & cleans up a differnt method.

Comment on attachment 133833 [details] [diff] [review] david's patch adding review? since I assume that's what you're looking for here. - (float)splitView:(NSSplitView *)sender constrainMinCoordinate:(float)proposedCoord ofSubviewAt:(int)offset { - const int kMinimumContainerSplitWidth = 150; - float retVal = proposedCoord; if ( sender == mContainersSplit ) - retVal = kMinimumContainerSplitWidth; - return retVal; + return 150; //minimum size of collections pane + else + return proposedCoord; Dave, I don't like that you removed the definition for the value there. I think it would be better to keep avoid hard-coding a number like that 150 into the code. The way it was before was maybe not ideal either, but what you did is worse I think. + mRootBookmarks = [[[BookmarkManager sharedBookmarkManager] rootBookmarks] retain]; + if (!mRootBookmarks) + return; Just return? Shouldn't you log something, complain, beep(), etc? I've got the patch installed and it works. If you fix those two issues in the code, I'll review+

Added CC for david. David, please have a look at my comment #2 http://bugzilla.mozilla.org/show_bug.cgi?id=223209#c2

updated. didn't need the check for mBookmarkManager, so I pitched it. And I added back in the const int rather than just run with the 150.

Comment on attachment 134196 [details] [diff] [review] updated >+ if (searchFrame.size.height < 80) { >+ searchFrame.size.height = 80; >+ bookmarkFrame.size.height -= 80; Could this 80 be #defined somwhere ?

I #defined the 80 & 150 in the header.

#define bad. const long or enum good.

landed w/out #defines.