316835 - Disable the History toolbar button when about:history is the active window/tab

Status: RESOLVED FIXED

(Camino Graveyard :: Toolbars & Menus, defect)

Description

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

We disable view-source when it can't be used on a page, back/fwd/stop when they can't be used, etc.

We should do the same with the History toolbar icon when about:history is the active tab/window.  That would probably help a little with the "history is not a toggle" problem.

(We probably need to audit which buttons shouldn't be enabled when in any collection of the Bookmarks Manager and fix all of them.)

Comment 1

[Håkan Waara]

This is similar to the other 10 bugs about "disable X when about:Y is viewed". We should really just put together a list of all X, and then disable them all when an about:-URL is shown.  Preferably also have one bug for all of these cases.

Comment 2

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

Sam and I talked about this and decided one bug per issue is better, because they're not all going to be the same "quick fix" (and some have menu items that need disabling, too), and that way someone can tackle them one at a time if they wish.

I'll make them all dependencies of one, though, for easier tracking.

Comment 3

[Håkan Waara]

If there's a patch for any of the about:-issues, I think that should cover all about:-issues, since it's basically the same fix...

Comment 4

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

about:history and about:bookmarks are the only two about:urls we need to worry about, because they're not pages (well, about:logo falls under bug 309132, but the fix for that bug should catch it).

about:history is actually a further special case because it's not a toggle (thus this bug).  I doubt it's gonna be one big smooth patch....

Comment 5

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

De-meta-izing this bug; there's a new meta bug, and fixing this right/fully depends on either fixing bug 318931 or finding another way to tell what the active collection is.

And as for my last comment, the Bookmarks toolbar button will only be disabled when about:history or about:bookmarks are either the first url visited in a given tab or preceded only by about:blank (this currently happens, yay); it either opens or closes the Manager in all other cases (see also bug 235863 for what it does need to do).

Comment 6

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

Grr, sorry for the bugspam, but we also need to disable the menu item in this case, too (when history is the active collection).  If that's not easy to do when someone fixes this, file a new bug then ;)

Comment 7

[Chris Lawson (gone)]

Taking.

Comment 8

[Chris Lawson (gone)]

Might as well try to get this for 1.6 too, since the bug it depends on is still targeted at 1.6 and I'm planning to work on it.

Comment 9

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

Mass un-setting milestone per 1.6 roadmap.

Filter on RemoveRedonkulousBuglist to remove bugspam.

Developers: if you have a patch in hand for one of these bugs, you may pull the bug back to 1.6 *at that point*.

Comment 10

[Chris Lawson (gone)]

Not a [good first bug] considering its dependency, which is nasty, and I'm definitely not working on this.

Comment 11

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

Chris, can we fix the menu item and the toolbar button validation like we just did 335979?  We really only care whether the view is showing, not what the URL is.

Comment 12

[Chris Lawson (gone)]

Attachment: fix v1.0

To answer Smokey's comment 11, yes, we can do that. It's not *quite* as simple as it was in the other bug, but yeah, we can do that.

Comment 13

[Christopher Henderson]

Comment on attachment 408331 [details] [diff] [review]
fix v1.0

Remove these blank lines in the patch: 84, 115, 123.

// return (![self bookmarkManagerIsVisible] || [self canHideBookmarks]) &&

Lose these two boolean checks; that way the user can still click the button in about:history with a BM collection showing and get back to the History collection. Provides parity with the Show History menu item, too.

r+ with those changes.

Comment 14

[Chris Lawson (gone)]

Attachment: fix v1.01

with hendy's r comments addressed

Comment 15

[Stuart Morgan]

Comment on attachment 411619 [details] [diff] [review]
fix v1.01

sr=smorgan

Comment 16

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

Landed on cvs trunk.