Status of Find bar settings are difficult to discern in dark theme

VERIFIED FIXED in Firefox 57

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
a year ago
9 months ago

People

(Reporter: Greg, Assigned: dao)

Tracking

(Blocks: 1 bug)

57 Branch
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed)

Details

(Whiteboard: [reserve-photon-visual][p4])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Created attachment 8894120 [details]
Highlight All.png

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.78 Safari/537.36

Steps to reproduce:

activated the Find bar and selected "Highlight All".

See screenshot of Find bar. Can you tell whether "Highlight All" is enabled or not?


Actual results:

I couldn't tell whether any change occurred in the setting. Is "Highlight All" enabled or disabled? With Dark Theme, it's difficult to discern what state the setting is in.


Expected results:

As with Bug 1386919, Mozilla is making poor usability decisions with no consideration for people with visual disabilities. It should be clearly discernible whether a setting is enabled or not, and it is not in Dark Theme.
Component: Untriaged → Theme

Comment 1

a year ago
(In reply to Greg from comment #0)
> As with Bug 1386919, Mozilla is making poor usability decisions with no
> consideration for people with visual disabilities. It should be clearly
> discernible whether a setting is enabled or not, and it is not in Dark Theme.

I'm pretty sure this is simply a bug / regression. This is normal in Nightly. It would be helpful if you can clarify when exactly this broke, like by using mozregression ( https://mozilla.github.io/mozregression/ ), instead of assuming bad faith.
Flags: needinfo?(gwmfms6)
Whiteboard: [photon-visual][triage]
(Reporter)

Comment 2

a year ago
I believe the regression started in v57 with the "new" dark theme.

This issue is not as bad in v54, but even in v54 it is not as good a design as it should be. The "dark highlight" is much easier to see in v54, but it should be a "bright highlight" or even a color change to accommodate people with visual disabilities etc. So obviously, the situation in v57 is a regression, but I am also taking issue with the "intended" design of v57 and even v54.

You guys should be clearly indicating focus with "bright highlight" or changed color and not simply reducing a shade, like you are currently doing with focused tab, find bar settings, etc. This is my argument that you are describing as "bad faith."
Flags: needinfo?(gwmfms6)
Flags: qe-verify+
Priority: -- → P3
Whiteboard: [photon-visual][triage] → [reserve-photon-visual][p4]
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

a year ago
Blocks: 1325171
QA Contact: brindusa.tot

Updated

11 months ago
Priority: P3 → P4

Updated

11 months ago
Duplicate of this bug: 1392772

Updated

11 months ago
Duplicate of this bug: 1394067

Updated

11 months ago
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox57: --- → affected
status-firefox-esr52: --- → unaffected

Comment 5

11 months ago
Looks like the findbar is missing the [brighttext] attribute to make this rule apply: https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/toolbarbuttons.inc.css#31

Updated

11 months ago
Assignee: nobody → dharvey

Updated

11 months ago
Status: NEW → ASSIGNED
Iteration: --- → 57.3 - Sep 19
Priority: P4 → P1
Comment hidden (mozreview-request)
(Assignee)

Comment 7

11 months ago
mozreview-review
Comment on attachment 8904964 [details]
Bug 1387762 - Ensure findbar is correctly styled in dark theme.

https://reviewboard.mozilla.org/r/176788/#review181734

::: browser/themes/shared/toolbarbuttons.inc.css:33
(Diff revision 1)
>  
>  :root[uidensity=touch] {
>    --toolbarbutton-inner-padding: 9px;
>  }
>  
> -toolbar[brighttext] {
> +:root:-moz-lwtheme-brighttext {

Unfortunately this is completely wrong for dark OS themes.
Attachment #8904964 - Flags: review?(dao+bmo) → review-
(Assignee)

Comment 8

11 months ago
I guess we could use both selectors, although this would still leave this bug unfixed for dark OS themes.

Comment 9

11 months ago
Why not add the findbar to this selector: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#8706 ?

Then make the CSS rule apply to findbar[brighttext]
(Assignee)

Updated

10 months ago
Assignee: dharvey → dao+bmo
Comment hidden (mozreview-request)
(Assignee)

Updated

10 months ago
Attachment #8904964 - Attachment is obsolete: true

Comment 11

10 months ago
mozreview-review
Comment on attachment 8905880 [details]
Bug 1387762 - Define default --toolbarbutton-hover-background and --toolbarbutton-active-background values that work on both light and dark backgrounds.

https://reviewboard.mozilla.org/r/177688/#review182824

Thats a pretty smart solution and works well here
Attachment #8905880 - Flags: review?(dharvey) → review+

Comment 12

10 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40df319376b8
Define default --toolbarbutton-hover-background and --toolbarbutton-active-background values that work on both light and dark backgrounds. r=daleharvey
https://hg.mozilla.org/mozilla-central/rev/40df319376b8
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Comment 14

10 months ago
I have reproduced this bug with Nightly 57.0a1 (2017-08-05) on Windows 8.1, 64-Bit ! 

This bug's fix is Verified with latest Nightly !

Build   ID    20170909100226
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0

[bugday-20170906]

Updated

10 months ago
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1402614

Updated

9 months ago
Depends on: 1414693
You need to log in before you can comment on or make changes to this bug.