1637846 - Reader view doorhanger uses 'active' colour for focused dropdown buttons; should match hover colour instead

Status: RESOLVED WORKSFORME

(Toolkit :: Reader Mode, defect, P3)

Description

[Asif Youssuff]

Attachment: Screenshot from 2020-05-13 23-53-37.png

Steps to reproduce:

1. Go to https://getpocket.com/explore/item/who-still-buys-wite-out-and-why
2. Enable reader view
3. click any of increase/decrease font size, increase/decrease content width, and increase/decrease line height

What happens:

The active focus color gets stuck to the button as I hover over other buttons.

Expected result:

The active color should only be active momentarily.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Hi akshitha! Any chance you might have some time to look at this?

Comment 2

[:Gijs (away/busy until Mar 10)]

FWIW, I cannot reproduce this on macOS or Windows. Are you seeing this on Linux?

Comment 3

[Asif Youssuff]

Yes, it is happening in Linux, and in all color modes - I hadn't noticed previously because it is less obvious.

Comment 4

[:Gijs (away/busy until Mar 10)]

(In reply to Asif Youssuff from comment #3)

Yes, it is happening in Linux, and in all color modes - I hadn't noticed previously because it is less obvious.

So I just managed to repro on Windows - but only after using keyboard focus to move around the document. If I close it and open the same article another time, and don't use the [tab] key, the problem goes away again. The styling is wrong, and should match :hover, not :active styling.

However, I also think the distinction between Linux and Windows is wrong here. Emilio, why does :-moz-focusring apply on Linux when the keyboard has not been used to navigate the document?

Comment 5

[:Gijs (away/busy until Mar 10)]

Talked on matrix with Emilio. Filed bug 1638127 for the Linux focus behaviour.

Comment 6

[:Gijs (away/busy until Mar 10)]

Sigh, bugzilla, at no point did I mark this fixed.

Comment 7

[:Gijs (away/busy until Mar 10)]

Though I am not really sure whether there's any value in keeping both this and bug 1637847 open, they feel pretty interrelated.

Comment 8

[:Gijs (away/busy until Mar 10)]

(In reply to :Gijs (he/him) from comment #7)

Though I am not really sure whether there's any value in keeping both this and bug 1637847 open, they feel pretty interrelated.

Hm, though I think this bug might be fixed as a result of the design changes in bug 1637652, and bug 1637847 isn't fixed yet. Need to doublecheck.

Comment 9

[Mathew Hodson]

Attachment: Screenshot of menu showing the background color changed for two buttons

I can still reproduce on Nightly build 20200523092124 if one of the buttons gets focus.

Comment 10

[Mathew Hodson]

This is a regression from bug 1550836. It doesn't reproduce on Firefox 77.

https://hg.mozilla.org/mozilla-central/rev/1281f97e9ddd#l9.571 added focusring selectors to the CSS that changes the button color. Now, buttons get the hover color both when they hovered and when they are focused. Before, they only got the color when they were hovered.

Comment 11

[:Gijs (away/busy until Mar 10)]

(In reply to Mathew Hodson from comment #9)

Created attachment 9151285 [details]
Screenshot of menu showing the background color changed for two buttons

I can still reproduce on Nightly build 20200523092124 if one of the buttons gets focus.

Sorry, but I think I'm looking at this screenshot differently. Using active styling (ie the same as when you've got a mousebutton down on top of a button) for focus would be incorrect, but your screenshot shows hover styling. So this bug as filed is fixed - in dark mode you wouldn't get the white styling shown in the screenshot from comment #0. (I also tested locally on macOS to confirm.)

Using the same styling for hover and focus was sort of intentional - if you're using keyboard focus, having a nicer visual focus indicator than the default one is a feature, not a bug. Of course, having it be the same as the hover styling can be confusing, and we could morph this bug a second time and make it about that... but that doesn't seem hugely important to me, especially now that bug 1638127 is fixed, so (even on Linux...) you'll only see the focus styling if you're deliberately using keyboard focus to engage with the buttons and dropdown, in which case you're probably less likely to use the mouse at the same time anyway.

Matthew, am I missing something here?

Comment 12

[Matthew Hodgson]

(In reply to :Gijs (he/him) from comment #11)
"Mathew Hodson" != "Matthew Hodgson" ;)

Comment 13

[Mathew Hodson]

(In reply to :Gijs (he/him) from comment #11)

Sorry, but I think I'm looking at this screenshot differently. Using active styling (ie the same as when you've got a mousebutton down on top of a button) for focus would be incorrect, but your screenshot shows hover styling. So this bug as filed is fixed - in dark mode you wouldn't get the white styling shown in the screenshot from comment #0. (I also tested locally on macOS to confirm.)

Using the same styling for hover and focus was sort of intentional

You're right I missed the distinction between active color and hover color.

If focus showing the hover color is intentional, then this was fixed by bug 1637993. That's when the new behavior shows up.

Comment 14

[:Gijs (away/busy until Mar 10)]

(In reply to Matthew Hodgson from comment #12)

(In reply to :Gijs (he/him) from comment #11)
"Mathew Hodson" != "Matthew Hodgson" ;)

I am mortified. Sorry to both of you!

(In reply to Mathew Hodson from comment #13)

If focus showing the hover color is intentional, then this was fixed by bug 1637993. That's when the new behavior shows up.

Alright. Let's close this bug out, and I've filed bug 1640505 about distinguishing focus/hover styling in these dropdowns.