1704195 - Hover-state of geolocation prompt is unreadable in dark/alpenglow themes and indistinguishable from unhovered in default theme

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P2)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Visit some site that triggers a geolocation prompt. E.g.: go to https://developer.mozilla.org/en-US/docs/Web/API/Geolocation_API#result and click the demo's Show my location button.

2. Hover the "Deny"/"Allow" buttons on the popup.

EXPECTED RESULTS:
(A) The hover state should be distinguishable from the unhovered state.
(B) The text should still be readable in the hover state.

ACTUAL RESULTS:
Varying results depending on your Firefox theme:

• With the "Default" Firefox theme, we violate expectation (A) -- the hover state is only barely distinguishable from the un-hovered state. (I think the text color just has an extremely subtle change.)
• With the "Dark" and "Alpenglow" Firefox themes, we violate expectation (B) -- the text is unreadable in the hovered state (black text on a dark background)
• With the "Light" Firefox theme, we live up to both expectations.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot with default theme (I'm hovering the "Block" button. Can you tell that it's hovered?)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot with "Dark" theme (I'm hovering "Block" button, and it becomes hard to read)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: screenshot with "Alpenglow" theme (I'm hovering the Block button, and it's hard to read)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: screenshot with "Light" theme (this one seems fine; I'm hovering "Block", and it looks hovered and is still readable)

Comment 5

[Daniel Holbert [:dholbert]]

mozregression gives me this regression range:
https://hg.mozilla.org/integration/autoland/rev/44916257e3052502b8d9465b73be4d5a8c4919e7
--> regressed by bug Bug 1703716

(Before that, this popup has its "pre-proton" doorhanger UI, which isn't affected by this usability issue.)

Specifically looks like this would probably be under the umbrella of "browser.proton.doorhangers.enabled" whose value was flipped in that commit.

Comment 6

[Daniel Holbert [:dholbert]]

I'm using the latest Linux Nightly, FWIW.

[Tracking Requested - why for this release]: usability regression, with our default theme as well as other themes that we ship with (all but the non-default "light" theme)

Comment 7

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

Hey dao, are our dark theme fallbacks on Linux not doing what we expect here?

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1703716

Comment 9

[Ciprian Georgiu, Desktop QA]

This can be also seen in the Customize page on Ubuntu 18.04 x64, with latest Nightly 89.0a1, while hovering over the Themes list.

Comment 10

[Tim Nguyen :ntim]

Not sure about comment 1. For comment 2 & comment 3, I suspect https://searchfox.org/mozilla-central/source/toolkit/themes/linux/global/button.css#31-42 has too much specificity.

You could try changing button:hover:not(:active,[disabled="true"],[open="true"],[checked="true"],[default="true"]) { to button:where(:hover:not(:active,[disabled="true"],[open="true"],[checked="true"],[default="true"])) { which will reduce the specificity to 0.

Same for the active state. (I suspect the disabled state might be an issue on all platforms too).

Optional cleanup, but macOS could get the same treatment here: https://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/button.css#16,22,28 and then https://searchfox.org/mozilla-central/source/toolkit/themes/shared/popupnotification.inc.css#196,201 could get simplified (no !important on the second declaration, and the first declaration removed).

Molly made a related fix for Windows in https://phabricator.services.mozilla.com/D112103.

Comment 11

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

Thanks for the guidance, ntim!

Comment 12

[:Gijs (he/him)]

Looks like Dão has explicitly asked for the suggestion from comment #10 in bug 1707627 so that bug should fix this one too, AIUI, so marking a dependency.

Comment 13

[Tim Nguyen :ntim]

I think bug 1708690 might have fixed this. Daniel, can you confirm?

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

Looks like Dão has explicitly asked for the suggestion from comment #10 in bug 1707627 so that bug should fix this one too, AIUI, so marking a dependency.

Bug 1707627 is slightly different, it's about toolbarbutton.css, not button.css.

Comment 14

[Daniel Holbert [:dholbert]]

Yup, this is fixed! Default, Dark, and Alpenglow all seem good now (they don't violate expectations as described in comment 0).

mozregression says the fix range is https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=27780a561fa02a7df9f3068f3f1d6dadde083c87&tochange=02c956ff0d2f2c3584939f6d1879dad39795738d

So yes, fixed by bug 1708690. I'll close as FIXED by that bug.

Comment 15

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 89.0a1 (2021.04.09) on Ubuntu 20.04 x64.
I can confirm this issue is fixed, I verified using Firefox 89.0 and Fx 90.0a1 on Ubuntu 20.04 x64, macOS 11.0.1 and Win 8.1.04 x64.