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

VERIFIED FIXED in Firefox 60

Status

()

defect
P4
normal
VERIFIED FIXED
2 years ago
11 months ago

People

(Reporter: itiel_yn8, Assigned: Gijs)

Tracking

unspecified
Firefox 60
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox57 wontfix, firefox58 wontfix, firefox59 wontfix, firefox60 verified)

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(5 attachments)

Reporter

Description

2 years ago
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.
Assignee

Comment 1

2 years ago
(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?
Flags: needinfo?(itiel_yn8)
Reporter

Comment 2

2 years ago
(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.
Flags: needinfo?(itiel_yn8)
Reporter

Comment 3

2 years ago
Posted image Demo
Reporter

Comment 4

2 years ago
Reporter

Comment 5

2 years ago
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
Reporter

Updated

2 years ago
Has Regression Range: --- → yes
Keywords: regression
Assignee

Comment 6

2 years ago
(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 ).
Flags: needinfo?(itiel_yn8)
Assignee

Updated

2 years ago
Blocks: 1354126
Priority: -- → P4
Whiteboard: [reserve-photon-structure]
Reporter

Comment 7

2 years ago
(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.
Flags: needinfo?(itiel_yn8)
Reporter

Comment 8

2 years ago
Posted image Demo #2
Assignee

Comment 9

2 years ago
(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.
Reporter

Comment 10

2 years ago
(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.
Reporter

Comment 11

2 years ago
Posted image GIF
Assignee

Comment 12

2 years ago
(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?
Flags: needinfo?(abenson)

Comment 13

2 years ago
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.
Flags: needinfo?(abenson)
Assignee

Updated

2 years ago
Summary: "Toolbars", "Themes" and "Density" buttons in the customization mode should lose focus when the user is done with them → Stop using hover (darker) styling for :focus of "Toolbars", "Themes" and "Density" buttons in customization mode
Flags: qe-verify+
QA Contact: gwimberly
Reporter

Updated

2 years ago
Reporter

Updated

2 years ago
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).
Assignee

Comment 15

a year ago
Meh, let's just fix it.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
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
Attachment #8946861 - Flags: review?(jaws) → review+

Comment 18

a year ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf1ed01c25ea
stop giving buttons in customize mode a darker background on focus, r=jaws

Comment 19

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bf1ed01c25ea
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Reporter

Comment 20

a year ago
Fixed on latest Nightly.
Status: RESOLVED → VERIFIED
Reporter

Updated

a year ago
You need to log in before you can comment on or make changes to this bug.