The default bug view has changed. See this FAQ.

Remove a pre-panther check

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
General
--
trivial
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: froodian (Ian Leue), Assigned: peeja)

Tracking

({fixed1.8.1})

Trunk
Camino1.5
PowerPC
Mac OS X
fixed1.8.1

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
http://lxr.mozilla.org/mozilla/source/camino/src/browser/BrowserWindowController.mm#934
Let's get a patch up for this and get it off the list ;)
(Assignee)

Comment 2

11 years ago
Created attachment 233668 [details] [diff] [review]
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.
Attachment #233668 - Flags: review?(nick.kreeger)
(Reporter)

Updated

11 years ago
Assignee: nobody → peter.a.jaros
>  // 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?
(Assignee)

Comment 4

11 years ago
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:.
Status: NEW → ASSIGNED

Comment 5

11 years ago
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.
Attachment #233668 - Flags: review?(nick.kreeger) → review+
(Reporter)

Updated

11 years ago
Attachment #233668 - Flags: superreview?(mikepinkerton)
(Assignee)

Comment 6

11 years ago
(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 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.
Attachment #233668 - Flags: superreview?(mikepinkerton) → superreview+
(Reporter)

Comment 8

11 years ago
Created attachment 235133 [details] [diff] [review]
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.
Attachment #233668 - Attachment is obsolete: true
(Reporter)

Updated

11 years ago
Whiteboard: [needs checkin]

Comment 9

11 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [needs checkin]
You need to log in before you can comment on or make changes to this bug.