328625 - Fix missing casts and remove some dead code from bookmarks code

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Chris Lawson (gone)]

Getting several compiler warnings in our bookmarks code, and there's a huge block of validation code commented out in BookmarkViewController.mm. The forthcoming patch fixes this stuff, at least in part.

Comment 1

[Chris Lawson (gone)]

Attachment: fixes several casts, a spelling error, and removes unused validation code

Comment 2

[Stuart Morgan]

Comment on attachment 213226 [details] [diff] [review]
fixes several casts, a spelling error, and removes unused validation code

Looks fine and still builds.
r=me

Comment 3

[Chris Lawson (gone)]

Comment on attachment 213226 [details] [diff] [review]
fixes several casts, a spelling error, and removes unused validation code

sr?=mento

Comment 4

[Mark Mentovai]

Comment on attachment 213226 [details] [diff] [review]
fixes several casts, a spelling error, and removes unused validation code

>Index: mozilla/camino/src/bookmarks/BookmarkViewController.mm
>===================================================================
>@@ -771,17 +771,17 @@
> // XXX unused
> -(void) displayBookmarkInOutlineView:(BookmarkItem *)aBookmarkItem

OK.  Are we ever going to kill abandoned code?  #if 0 it out?

>@@ -841,17 +841,17 @@
> 
>   if ([mBookmarksOutlineView numberOfSelectedRows] == 1)
>   {
>     BookmarkItem *item = [self selectedBookmarkItem];
>     // if it's a folder, use it
>     if ([item isKindOfClass:[BookmarkFolder class]])
>     {
>       *outIndex = (int)[(BookmarkFolder*)item count];
>-      return item;
>+      return (BookmarkFolder*)item;

It's unclean to cast to the same type twice in a row.  How about:

  BookmarkFolder* selectedFolder = (BookmarkFolder*) item;
  *outIndex = [selectedFolder count];
  return selectedFolder;

sr=me, as long as "s" means "second" :)

Comment 5

[Chris Lawson (gone)]

Comment on attachment 213226 [details] [diff] [review]
fixes several casts, a spelling error, and removes unused validation code

Bitrotted.

Comment 6

[Chris Lawson (gone)]

Attachment: updated to fix bitrot + mento's comment, ready for checkin

Comment 7

[Mark Mentovai]

Checked in, trunk and 1_8.  Thanks for the cleanup patch.

Your second patch was so much smaller than the first, so I looked at the diff-to-diff diff and also fixed the "its" -> "it's" comment.  For validateMenuItem:, I turned the /* */ into an #if 0 block.  #if 0.  The better way to kill.  Bugs check in, but they don't check out.