1706155 - The Buttons on modals do not change their state on hover or Button press when the High contrast mode is enabled

Status: VERIFIED FIXED

(Toolkit :: Themes, defect, P2)

Description

[Rares Doghi, Desktop QA]

Attachment: desktopHover.png

[Affected platforms]:
Platforms: All

[Steps to reproduce]

1. Enable High Contrast on any platform.
2. Trigger any modal.
3. Hover or Click any button from the Modal.

[Expected result]
The button from the modals should change their state when the user hovers over them or holds the Click on them.

[Actual result]
The buttons from the modals do not change their state at all when the user hovers over them or clicks them with High contrast enabled.
Please note that this issue occurs on all platforms , Windows, Mac and Ubuntu.

Please also note that a similar modal like the Update panels have the correct behavior with High contrast enabled. Maybe that helps.

Comment 1

[Rares Doghi, Desktop QA]

Not sure how helpful the screenshot is but I was hovering on top of the "Not now" button and there is no difference between normal or hover.

Comment 2

[:Gijs (he/him)]

(In reply to Rares Doghi from comment #0)

Please also note that a similar modal like the Update panels have the correct behavior with High contrast enabled. Maybe that helps.

Sorry, which panel do you mean by this?

Comment 3

[Rares Doghi, Desktop QA]

Attachment: HighContrast_download_panel.png

Its the update panels like this one.

Comment 4

[:Gijs (he/him)]

(In reply to Rares Doghi from comment #3)

Created attachment 9216982 [details]
HighContrast_download_panel.png

Its the update panels like this one.

AFAICT these just don't adjust to HCM at all, and always use hardcoded colours, which is the problem I tried to fix for modals and infobars in bug 1705133.

I think I have an idea on how we can do better for the modals/infobars case and I had to look into HCM anyway for bug 1700517, so I'll see if I can make that work quickly...

Mike, do you know if for d--rhangers / appmenu, there was a deliberate choice to keep custom colours in HCM, or if not, if a bug is already on file for that?

Comment 5

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

With HCM, which I believe matches on (prefers-contrast), we explicitly don't set any custom colours: https://searchfox.org/mozilla-central/rev/f018480dfed4fc583703a5770a6db9ab9dc0fb99/browser/themes/shared/browser-custom-colors.inc.css#5

With HCM in our panels, it falls back to system colors - MenuText for the text, and -moz-accent-color-foreground and -moz-accent-color for the default button. The secondary buttons end up with these fallbacks, which are colour mixes based off of the currentColor (MenuText): https://searchfox.org/mozilla-central/rev/f018480dfed4fc583703a5770a6db9ab9dc0fb99/browser/themes/shared/browser.inc.css#24-27

The common.inc.css rules that apply to modals seem different... we might want to do something similar in common.inc.css to what we're doing in panels, as I'm reasonably sure it's based off of what we were doing pre-Proton.

Comment 6

[Mark Striemer [:mstriemer]]

Attachment: Bug 1706155 - Set hover states for HCM common.css

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a02c8fa7821
Set hover states for HCM common.css r=desktop-theme-reviewers,dao,morgan,preferences-reviewers

Comment 8

[Andreea Pavel [:apavel]]

Backed out for bc failures at /browser_parsable_css.js

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=Mb7ughjwRZWMRbp12X9Rvg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=8a02c8fa782141b57671588f22995efa09627a73

Failure log: https://treeherder.mozilla.org/logviewer?job_id=339106030&repo=autoland&lineNumber=2835

Backout: https://hg.mozilla.org/integration/autoland/rev/12675d6ca9929effe8d8f69d49c1f432447de120

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8757d112fc04
Set hover states for HCM common.css r=desktop-theme-reviewers,dao,morgan,preferences-reviewers

Comment 10

[Andreea Pavel [:apavel]]

Backed out for bc failures at browser_parsable_css.js

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=TmCxl477R2WM2VdSq3aOTg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=8757d112fc04045e4b2e918072664e4a6fc2bbf5

Failure log: https://treeherder.mozilla.org/logviewer?job_id=339115170&repo=autoland&lineNumber=2832

Backout: https://hg.mozilla.org/integration/autoland/rev/8a9296bfb1603d9739b6f323f34f8e3c16c1ad7b

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e8c89be4b84
Set hover states for HCM common.css r=desktop-theme-reviewers,dao,morgan,preferences-reviewers

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2e8c89be4b84

Comment 13

[Rares Doghi, Desktop QA]

Hi Mark, can you please take a look at my screen recording and screenshots ? it seems that for the Protocol Handlers modal the Checkbox dissapears when highlighted, its very weird since on other modals the checkbox behaves correctly.
Theres also an issue with the Tag Arrow from the Add/Edit Bookmark modal, the same thing when hovering over it it dissapears/appears.

Comment 14

[Rares Doghi, Desktop QA]

Attachment: SnoopDoggContrast.mp4

Comment 15

[Rares Doghi, Desktop QA]

Attachment: BookmarkTagArrow.png

Comment 16

[:Gijs (he/him)]

Guessing you really meant Mark and not Mike. :-)

Comment 17

[Rares Doghi, Desktop QA]

After reading Comment 5 I thought Mike is also working on this and Mark was already need infoed so I just added Mike as well :))

Comment 18

[Mark Striemer [:mstriemer]]

Do you mind filing those bugs separately?

The bookmarks arrow includes comments about the CSS rules being there to support Windows 8 HCM so worth investigating that a little extra I think. Also the bookmarks one for me I reproduced by focusing a regular button, is that what you saw? So clicking the bookmarks tag arrow would make it disappear, but then tabbing down to the Cancel button would also make the Cancel button's text disappear.

Comment 19

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest Nightly build and I have logged Bug 1710852 for the remaining issue.

Comment 20

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #30072 (as of Thu, 13 May 2021 17:19:37 GMT) ==

Improvements:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
12%	build times		linux64	base-toolchains-clang taskcluster-c5d.4xlarge	1,380.81 -> 1,214.30

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=30072

Comment 21

[:Gijs (he/him)]

(In reply to Alexandru Ionescu (needinfo me) [:alexandrui] from comment #20)

== Change summary for alert #30072 (as of Thu, 13 May 2021 17:19:37 GMT) ==

Improvements:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
12%	build times		linux64	base-toolchains-clang taskcluster-c5d.4xlarge	1,380.81 -> 1,214.30

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=30072

FWIW, I suspect this is misattributed (or if it's not, I'd really like to understand how this is possible!) - this added some CSS and made some minute markup changes, but no build file changes or anything like that...

Comment 22

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Did some retriggers, thanks!

Comment 23

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Reassigned, thanks!