907376 - Disable the Exit button when in Customization mode

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike de Boer [:mikedeboer]]

Since bug 871203 landed, we have an Exit button in the panel. When in Customization Mode, this button might appear to (falsely) suggest it'll take you out of that mode, while instead it will close the browser.

To circumvent misinterpretations of this feature and remedy confusion, the exit button should be disabled.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: disable Exit button in Customization Mode

Comment 2

[Mike de Boer [:mikedeboer]]

http://hg.mozilla.org/projects/ux/rev/6f8a964f1294

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

http://mxr.mozilla.org/projects-central/source/ux/browser/themes/shared/customizableui/panelUIOverlay.inc.css#239 should have been updated to also reference #PanelUI-quit[disabled].

Comment 4

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2.1: add opacity rule for disabled Exit button

Forgot to add this rule. Jared, thanks for finding that!

/me slaps himself for missing this

Comment 5

[:Gijs (out for now; he/him)]

Comment on attachment 793417 [details] [diff] [review]
Patch v2.1: add opacity rule for disabled Exit button

Review of attachment 793417 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 6

[:Gijs (out for now; he/him)]

(In reply to :Gijs Kruitbosch from comment #5)
> Comment on attachment 793417 [details] [diff] [review]
> Patch v2.1: add opacity rule for disabled Exit button
> 
> Review of attachment 793417 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me

Actually, nit: please add the rule above the existing one so as not to break blame.

Comment 7

[:Gijs (out for now; he/him)]

FTFM

(In reply to :Gijs Kruitbosch from comment #6)
> Actually, nit: please add the *selector* above the existing one so as not to break
> blame.

Comment 8

[Mike de Boer [:mikedeboer]]

I can't (In reply to :Gijs Kruitbosch from comment #6)
> Actually, nit: please add the rule above the existing one so as not to break
> blame.

I can't do that. I've never heard of rule that should break code-consistency over supporting lack of intelligence in tooling. #PanelUI-help is ALWAYS above #PanelUI-quit in that file, so to break that consistency for the sake of tooling is not gonna fly.

As a side note, blame is preserved in VCS revision history. In the current toolchain (like hg.mozilla.org code browser, annotation view), it's a couple of clicks extra to check out the parent revision and blame from there. It might be less convenient.

Comment 9

[:Gijs (out for now; he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #8)
> I can't (In reply to :Gijs Kruitbosch from comment #6)
> > Actually, nit: please add the rule above the existing one so as not to break
> > blame.
> 
> I can't do that. I've never heard of rule that should break code-consistency
> over supporting lack of intelligence in tooling. #PanelUI-help is ALWAYS
> above #PanelUI-quit in that file, so to break that consistency for the sake
> of tooling is not gonna fly.

Well, I couldn't really see that from the patch context. I also don't think that consistency is particularly important, but I can't bring myself to care much either way. Land it whichever way around you think is best.

Comment 10

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs Kruitbosch from comment #9)
> Well, I couldn't really see that from the patch context. I also don't think
> that consistency is particularly important, but I can't bring myself to care
> much either way. Land it whichever way around you think is best.

Okido, thanks!

Comment 11

[Mike de Boer [:mikedeboer]]

http://hg.mozilla.org/projects/ux/rev/7a784c538b5c

Comment 12

[:Gijs (out for now; he/him)]

https://hg.mozilla.org/mozilla-central/rev/6f8a964f1294
https://hg.mozilla.org/mozilla-central/rev/7a784c538b5c