345552 - Remove a pre-panther check

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[froodian (Ian Leue)]

http://lxr.mozilla.org/mozilla/source/camino/src/browser/BrowserWindowController.mm#934

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Let's get a patch up for this and get it off the list ;)

Comment 2

[Peter Jaros (:peeja)]

Attachment: removes old panther checks, SearchToolbarItemIdentifier, mBookmarkToolbarItem, and mSidebarToolbarItem in BWC

Cleaned up a lot of old code.  Not just the Panther checks, but also references to SearchToolbarItemIdentifier that are now defunct, and mBookmarkToolbarItem and mSidebarToolbarItem, since they were not only redundant, but never used.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

>  // toolbarWillAddItem: (toolbar delegate method)
>  //
>  // Called when a button is about to be added to a toolbar. This is where we should
> -// cache items we may need later. For instance, we want to hold onto the sidebar
> -// toolbar item so we can change it when the drawer opens and closes.

Won't that still be true (be needed) when we get the Bookmarks toolbar button changing state depending on the state of the Manager?  Or are we "holding on" to it for another reason entirely there?

Comment 4

[Peter Jaros (:peeja)]

Well, mBookmarkToolbarItem and mSidebarToolbarItem are never read from in the entire project, and the changes seemed not to break the logic that chages the Bookmarks button's state.  The state changing should be handled by validateToolbarItem:.

Comment 5

[Nick Kreeger]

Comment on attachment 233668 [details] [diff] [review]
removes old panther checks, SearchToolbarItemIdentifier, mBookmarkToolbarItem, and mSidebarToolbarItem in BWC

+// cache items we may need later.
+// (void)toolbarWillAddItem:(NSNotification *)notification
+//{
+//  (Nothing needed at the moment.)
+//}

Whats the point of keeping this?

Looks OK, I doubt that this is all the panther checking code that we have.

Comment 6

[Peter Jaros (:peeja)]

(In reply to comment #5)

> Whats the point of keeping this?

I'm happy to get rid of it, but in IRC people said to be wary of removing methods.  Since the method would be empty, I thought this was a compromise. :)

Of course I'm happy to defer to a higher ruling.

Comment 7

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 233668 [details] [diff] [review]
removes old panther checks, SearchToolbarItemIdentifier, mBookmarkToolbarItem, and mSidebarToolbarItem in BWC

sr=pink

i just want to make sure this doesn't break people upgrading from 1.0 where that search field key is used in the toolbar plist.

Comment 8

[froodian (Ian Leue)]

Attachment: This is the bitrot police

I tested this out going from a clean 1.0 profile to a build with this patch in it with no ill consequences.  It had however, bitrotted (from my whitespace fixes).  Since peter is sadly bereft of a machine capable of building, I'm unbitrotting for him.

Comment 9

[Håkan Waara]

Checked in.