Closed
Bug 1513401
Opened 5 years ago
Closed 5 years ago
Graphite text highlighting does not have enough contrast on light background
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
VERIFIED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox64 | --- | unaffected |
firefox65 | + | verified |
firefox66 | + | verified |
People
(Reporter: lizzard, Assigned: ntim)
References
Details
(Keywords: qawanted, regression)
Attachments
(4 files)
With the 66 update, when you highlight or select text in a page, in a text field, or in the addressbar, the highlighting is barely visible.
Reporter | ||
Updated•5 years ago
|
status-firefox65:
--- → unaffected
status-firefox66:
--- → affected
OS: Unspecified → Mac OS X
Version: 65 Branch → 66 Branch
Reporter | ||
Comment 1•5 years ago
|
||
Not sure if this is limited to Mojave or whether it goes across other macOS versions.
Keywords: qawanted
Summary: macOS Mojave dark theme highlighting contrast is not visible enough → In 66 on macOS Mojave, highlighting contrast is not visible enough
Updated•5 years ago
|
Reporter | ||
Comment 3•5 years ago
|
||
Flags: needinfo?(lhenry)
Reporter | ||
Comment 4•5 years ago
|
||
only the bug number is highlighted here
Reporter | ||
Comment 5•5 years ago
|
||
In 64 I could see visual feedback of what is highlighted.
Assignee | ||
Comment 6•5 years ago
|
||
Thanks! Sounds like what I mentioned in bug 1486204 comment 59. I guess I can special case colors with 0% saturation and do some extra processing there, since multiplying up 0% saturation [0] won't do anything. [0]: https://searchfox.org/mozilla-central/rev/3160ddc1f0ab55d230c595366662c62950e5c785/widget/cocoa/nsLookAndFeel.mm#157
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
status-firefox64:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Comment 7•5 years ago
|
||
Yeah that sounds reasonable.
Assignee | ||
Updated•5 years ago
|
Flags: needinfo?(ntim.bugs)
Assignee | ||
Updated•5 years ago
|
Summary: In 66 on macOS Mojave, highlighting contrast is not visible enough → Graphite text highlighting does not have enough contrast on light background
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → ntim.bugs
Flags: needinfo?(ntim.bugs)
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
tracking-firefox65:
--- → ?
tracking-firefox66:
--- → ?
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2c2d55d510a7 Increase contrast of unsaturated selection colors in nsLookAndFeel.mm. r=mstange
Assignee | ||
Comment 10•5 years ago
|
||
Comment on attachment 9031462 [details] Bug 1513401 - Increase contrast of unsaturated selection colors in nsLookAndFeel.mm. r=mstange [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1486204 User impact if declined: Graphite selection color is faded out on light themes Is this code covered by automated tests?: No Has the fix been verified in Nightly?: No Needs manual test from QE?: Yes If yes, steps to reproduce: See previous comments List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Self contained widget/cocoa change String changes made/needed:
Attachment #9031462 -
Flags: approval-mozilla-beta?
Comment 11•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2c2d55d510a7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Comment 12•5 years ago
|
||
> + value = 255 - (255 - value) * factor;
I realized a problem in the math: if value is less that 128, this will be 255 - [something larger than 255], which will be negative and then wraparound into a large uint16_t value when assigned to "value". So I think the "(255 - value) * factor" part should be wrapped in a mozilla::clamped(..., 0, 255).
This is not that urgent to fix because macOS does not, as far as I know, have gray highlighting colors that are darker than 128. But it should still be fixed.
Assignee | ||
Comment 13•5 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #12) > > + value = 255 - (255 - value) * factor; > > I realized a problem in the math: if value is less that 128, this will be > 255 - [something larger than 255], which will be negative and then > wraparound into a large uint16_t value when assigned to "value". So I think > the "(255 - value) * factor" part should be wrapped in a > mozilla::clamped(..., 0, 255). > > This is not that urgent to fix because macOS does not, as far as I know, > have gray highlighting colors that are darker than 128. But it should still > be fixed. Filed bug 1514464, thanks!
Comment 14•5 years ago
|
||
Comment on attachment 9031462 [details] Bug 1513401 - Increase contrast of unsaturated selection colors in nsLookAndFeel.mm. r=mstange [Triage Comment] Fixes faded selection color when using a light theme on macOS. Approved for 65.0b5.
Attachment #9031462 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•5 years ago
|
Flags: qe-verify+
Comment 15•5 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/7cc9c0386e87
Comment 16•5 years ago
|
||
Hello all, This issue is verified fixed using Firefox 65.0b5 and the latest Nightly 66.0a1 (2018-12-17)on macOS 10.14(Mojave).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•5 years ago
|
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•