1391007 - Stop using hover (darker) styling for :focus of "Toolbars", "Themes" and "Density" buttons in customization mode

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P4)

Description

[Itiel]

STR:
1. Open Nightly > menu > Customize
2. Click on one of the "Toolbars", "Themes" and "Density" button
3. Click somewhere else (or the same button, again), to make the list disappear

ER:
These buttons should lose the focus when they're being closed.

AR:
They don't.

Comment 1

[:Gijs (he/him)]

(In reply to ItielMaN from comment #0)
> ER:
> These buttons should lose the focus when they're being closed.

Why? This would be very inconvenient for keyboard users, right? Where should focus go instead? Why is it moving at all?

Comment 2

[Itiel]

(In reply to :Gijs from comment #1)
> (In reply to ItielMaN from comment #0)
> > ER:
> > These buttons should lose the focus when they're being closed.
> 
> Why? This would be very inconvenient for keyboard users, right? Where should
> focus go instead? Why is it moving at all?

When I said "focus" I meant the visual focus on the buttons (like in hover state).
See attached "Example of good behaviour" for how it looks like in Settings, and "Demo" for how it looks in these buttons.

Comment 3

[Itiel]

Attachment: Demo

Comment 4

[Itiel]

Attachment: Example of good behaviour

Comment 5

[Itiel]

This one is a regression, 2017-05-24 to 2017-05-25:

2017-08-25T10:45:53: DEBUG : Starting merge handling...
2017-08-25T10:45:53: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=542bbb51c412a0f292b7e6d081f5d15563bfb06b&full=1
2017-08-25T10:45:54: DEBUG : Found commit message:
Bug 1354126 - update labeling and styling of footer buttons, r=mikedeboer

MozReview-Commit-ID: DFzb4gbmWYj

2017-08-25T10:45:54: INFO : The bisection is done.
2017-08-25T10:45:54: INFO : Stopped

Comment 6

[:Gijs (he/him)]

(In reply to ItielMaN from comment #5)
> This one is a regression, 2017-05-24 to 2017-05-25:
> 
> 2017-08-25T10:45:53: DEBUG : Starting merge handling...
> 2017-08-25T10:45:53: DEBUG : Using url:
> https://hg.mozilla.org/integration/autoland/json-
> pushes?changeset=542bbb51c412a0f292b7e6d081f5d15563bfb06b&full=1
> 2017-08-25T10:45:54: DEBUG : Found commit message:
> Bug 1354126 - update labeling and styling of footer buttons, r=mikedeboer
> 
> MozReview-Commit-ID: DFzb4gbmWYj
> 
> 2017-08-25T10:45:54: INFO : The bisection is done.
> 2017-08-25T10:45:54: INFO : Stopped

Are the buttons actually not focused before that change? Or do we not show the dotted focus outline or darker background, but the buttons *are* in fact focused (so when you hit [tab], for example, the next button gets focused and so on)? Because judging by the change there, that's what I think would happen.... (you can make mozregression quickly launch an older build by running: mozregression --launch 2017-05-23 ).

Comment 7

[Itiel]

(In reply to :Gijs (out until Sep 1, expect no replies to requests) from comment #6)
> Are the buttons actually not focused before that change? Or do we not show
> the dotted focus outline or darker background, but the buttons *are* in fact
> focused (so when you hit [tab], for example, the next button gets focused
> and so on)? Because judging by the change there, that's what I think would
> happen.... (you can make mozregression quickly launch an older build by
> running: mozregression --launch 2017-05-23 ).

In this build:
- After clicking one of these buttons, the darker background is gone when I'm clicking somewhere else, but the dotted outline is visible at this point
- I'm able to switch buttons by clicking the Tab/Shift-Tab buttons (and I see the dotted outline moving from the previously focused button to the next one)

In short, the only issue here (on latest Nightly) is that the darker background is visible even if clicked somewhere else. Switching between buttons using the Tab button works correctly.

See attached GIF for how it looks on this build.

Comment 8

[Itiel]

Attachment: Demo #2

Comment 9

[:Gijs (he/him)]

(In reply to ItielMaN from comment #7)
> (In reply to :Gijs (out until Sep 1, expect no replies to requests) from
> comment #6)
> > Are the buttons actually not focused before that change? Or do we not show
> > the dotted focus outline or darker background, but the buttons *are* in fact
> > focused (so when you hit [tab], for example, the next button gets focused
> > and so on)? Because judging by the change there, that's what I think would
> > happen.... (you can make mozregression quickly launch an older build by
> > running: mozregression --launch 2017-05-23 ).
> 
> In this build:
> - After clicking one of these buttons, the darker background is gone when
> I'm clicking somewhere else, but the dotted outline is visible at this point
> - I'm able to switch buttons by clicking the Tab/Shift-Tab buttons (and I
> see the dotted outline moving from the previously focused button to the next
> one)
> 
> In short, the only issue here (on latest Nightly) is that the darker
> background is visible even if clicked somewhere else. Switching between
> buttons using the Tab button works correctly.
> 
> See attached GIF for how it looks on this build.

OK, but I think that's on purpose. The dark background also follows the focus around when tabbing between these buttons, right, just like when they're hovered? That's intentional.

Comment 10

[Itiel]

(In reply to :Gijs (out until Sep 1, expect no replies to requests) from comment #9)
> OK, but I think that's on purpose. The dark background also follows the
> focus around when tabbing between these buttons, right, just like when
> they're hovered? That's intentional.

If that was the case, every similar button in about:preferences should have acted the same.
In the attached GIF you can see that switching between the buttons ("Show Homepage" and "Use current pages") doesn't make the dark background follow from one to another, and the dark background is gone when pressing elsewhere.

Comment 11

[Itiel]

Attachment: GIF

Comment 12

[:Gijs (he/him)]

(In reply to Itiel from comment #10)
> (In reply to :Gijs (out until Sep 1, expect no replies to requests) from
> comment #9)
> > OK, but I think that's on purpose. The dark background also follows the
> > focus around when tabbing between these buttons, right, just like when
> > they're hovered? That's intentional.
> 
> If that was the case, every similar button in about:preferences should have
> acted the same.
> In the attached GIF you can see that switching between the buttons ("Show
> Homepage" and "Use current pages") doesn't make the dark background follow
> from one to another, and the dark background is gone when pressing elsewhere.

Sure, the question is whether the background should be used for the :focus state or whether just the dotted outline is sufficient. No focus state is defined in the design. Aaron, thoughts?

Comment 13

[Aaron Benson]

The dark background should only be 'active' when the menu's panel is open. The dotted lines are a seperate indicator that shows which UI element is currently 'selected' in the tab order and is fine if that were to persist. FWIW, what I'm seeing in Nighly on macOS looks like the correct behavior.

Comment 14

[Mike Taylor [:miketaylr]]

Setting to fix-optional and removing regression tracking flag, given P4 status. (We don't need to update these flags for every new release, FWIW).

Comment 15

[:Gijs (he/him)]

Meh, let's just fix it.

Comment 16

[:Gijs (he/him)]

Attachment: Bug 1391007 - stop giving buttons in customize mode a darker background on focus,

Review commit: https://reviewboard.mozilla.org/r/216748/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216748/

Comment 17

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

Comment on attachment 8946861 [details]
Bug 1391007 - stop giving buttons in customize mode a darker background on focus,

https://reviewboard.mozilla.org/r/216748/#review223024

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf1ed01c25ea
stop giving buttons in customize mode a darker background on focus, r=jaws

Comment 19

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/bf1ed01c25ea

Comment 20

[Itiel]

Fixed on latest Nightly.