Closed Bug 1706155 Opened 2 months ago Closed 1 month ago

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

Categories

(Toolkit :: Themes, defect, P2)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox88 --- disabled
firefox89 --- wontfix
firefox90 --- verified

People

(Reporter: rares.doghi, Assigned: mstriemer)

References

(Blocks 4 open bugs)

Details

(Whiteboard: [proton-modals] [proton-infobars] [priority:2a])

Attachments

(5 files)

Attached image 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.

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.

(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?

Flags: needinfo?(rares.doghi)
Whiteboard: [proton-modals][proton-infobars]

Its the update panels like this one.

Flags: needinfo?(rares.doghi)

(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?

Flags: needinfo?(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.

Flags: needinfo?(mconley)
Priority: -- → P2
Whiteboard: [proton-modals][proton-infobars] → [proton-modals] [proton-infobars] [priority:2a]
Assignee: nobody → mstriemer
Status: NEW → ASSIGNED
Depends on: 1708690
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8a02c8fa7821
Set hover states for HCM common.css r=desktop-theme-reviewers,dao,morgan,preferences-reviewers
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8757d112fc04
Set hover states for HCM common.css r=desktop-theme-reviewers,dao,morgan,preferences-reviewers
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2e8c89be4b84
Set hover states for HCM common.css r=desktop-theme-reviewers,dao,morgan,preferences-reviewers
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

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.

Flags: needinfo?(mconley)
Attached video SnoopDoggContrast.mp4
Attached image BookmarkTagArrow.png

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

Flags: needinfo?(mconley)

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 :))

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.

Flags: needinfo?(mstriemer) → needinfo?(rares.doghi)

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

Status: RESOLVED → VERIFIED
Flags: needinfo?(rares.doghi)
Regressions: 1711370

(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...

Flags: needinfo?(aionescu)

Did some retriggers, thanks!

Reassigned, thanks!

Flags: needinfo?(aionescu)
Regressions: 1712857
You need to log in before you can comment on or make changes to this bug.