Closed Bug 1531338 Opened 1 year ago Closed 1 year ago

Current permission for block autoplay is not visible in Site information panel with high contrast enabled

Categories

(Core :: Widget: Gtk, defect, P2)

66 Branch
Desktop
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- unaffected
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed
firefox69 --- fixed

People

(Reporter: dcicas, Assigned: stransky)

References

(Blocks 1 open bug)

Details

(Keywords: access, regression)

Attachments

(3 files)

Affected versions
*Fx 66.0b11

Affected platforms

  • Ubuntu 18.04 x64 LTS

Steps to reproduce

  1. Enable High Contrast in Ubuntu
  2. Open Firefox.
  3. Reach https://edition.cnn.com/videos
  4. Open the site information panel.
  5. Click on the autoplay Allow/Block drop down menu.

Expected result

  • The drop down menu displays the current setting and the Allow/Block settings

Actual result

  • The drop down menu only displays the Allow/Block, and the current setting is not visible because its black text on a black background.

Regression range

  • Will return ASAP with a regression range.

Seems more like a general issue with these menulists.

Component: Site Identity and Permission Panels → Theme
Component: Theme → Themes
Priority: -- → P3
Product: Firefox → Toolkit
Keywords: access
See Also: → 334462

I don't think this is a regression. I can reproduce the issue on Nightly from 2019-01-23 when the dropdown menu was first implemented.

I'd like to see us fix the bigger issue here, even if this doesn't block rollout of the feature.

Given that this affects accessibility of a set of menus, can you bump the priority up higher than P3?

Flags: needinfo?(dao+bmo)

Stransky or karlt, any idea how this should be fixed? See bug 334462 for a similar issue -- the patch there may have broken this, actually. I'm not sure that patch makes sense at all anymore for gtk3. Should we revert it?

Flags: needinfo?(stransky)
Flags: needinfo?(karlt)
Flags: needinfo?(dao+bmo)
Priority: P3 → P2

The same issue exists with data:text/html,<button>Press%20and%20Hold
I expect we want a -moz-buttonactivetext, but we only have a -moz-mac-buttonactivetext.

Flags: needinfo?(karlt)
See Also: → 1044595
Component: Themes → Widget: Gtk
Flags: needinfo?(stransky)
Priority: P2 → --
Product: Toolkit → Core

I see that was fixed on mac by hardcoding 0xff color to -moz-mac-buttonactivetext. (https://bug1044595.bmoattachments.org/attachment.cgi?id=8501083)

As the system Gtk+ theme colors can't be used here I expect we should do something similar on Linux/Gtk+.

Mac has basically only one system theme to deal with, Gtk doesn't... I don't see how this could be solved by hardcoding some specific value.

Priority: -- → P2

I have almost working patch. It's a general issue with wrong button text color.

Assignee: nobody → stransky
  • Follow Gtk and get theme button text color directly from "button" CSS node instead of "button label"
  • Provide new -moz-gtk-buttonactivetext color for active/pressed button text color
  • Replace ButtonText color with -moz-gtk-buttonactivetext when it's appropriate
Keywords: checkin-needed

Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39e6fc05451a
[Linux/Gtk] Get and use Gtk theme text color for active/pressed button, r=dao,emilio

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Please request uplift to beta if you deem it appropriate.

Duplicate of this bug: 237534

It would be nice to have it in next ESR line but it also needs uplift of Bug 1554433. Emilio, is also okay to uplift it?

Flags: needinfo?(stransky) → needinfo?(emilio)

I think it should be ok to uplift that, yeah, it's a pretty automatic refactoring, and it only hides the value that was just introduced so it's not web-observable (and thus not really risky).

Let me know if you want me to do the request, but you can also do that :)

Flags: needinfo?(emilio)

Comment on attachment 9063907 [details]
Bug 1531338 - [Linux/Gtk] Get and use Gtk theme text color for active/pressed button, r=dao

Beta/Release Uplift Approval Request

  • User impact if declined: When high contrast Gtk theme is used (for accessibility for instance) various Firefox buttons are not readable - they're completely black. It affects Site autoplay permissions, customizations and more.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: - Enable HighContrast system Gtk theme in tweaks
  • Launch Firefox, check button colors are correct in Customize and site permissions page.
  • List of other uplifts needed: Bug 1554433
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Gtk/Linux theme bug only.
  • String changes made/needed: none
Attachment #9063907 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Comment on attachment 9063907 [details]
Bug 1531338 - [Linux/Gtk] Get and use Gtk theme text color for active/pressed button, r=dao

gtk theme fix, approved for 68.0b9

Attachment #9063907 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attached image SS on 86.0b9

Hello,
I investigated this issue on Fx 68.0b9 and on the latest Nightly 69.0a BuildID: 20190610214846. This issue is still occurring on both builds.

Flags: needinfo?(stransky)
Blocks: 1558743

(In reply to Daniel Cicas [:dcicas], Release QA from comment #21)

Created attachment 9071272 [details]
SS on 86.0b9

Hello,
I investigated this issue on Fx 68.0b9 and on the latest Nightly 69.0a
BuildID: 20190610214846. This issue is still occurring on both builds.

You're right. Looks like the latest changes in the patch actually broke the dropdown although other issues addressed by this patch are fixed. Filed as Bug 1558743.

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