1725146 - [macOS 10.13] Address bar text is not readable if Firefox Dark theme is used

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[Alexandru Trif, Desktop QA [:atrif]]

Attachment: macOS_adress.png

Affected versions

• 91.0 (20210804193234)
• 92.0b2 (20210810185524)
• 93.0a1 (20210810213316)

Affected platforms

• macOS 10.13

Preconditions

• Firefox Dark theme
• Blue Highlight color inside System Preferences -> General

Steps to reproduce

1. Open Firefox, write something in the address bar, and highlight it.

Expected result

• Text inside the Address bar is displayed as expected.

Actual result

• Text inside the Address bar is barely visible.

Regression range

• Last good revision: b8b54a4990d7e778b82909a871e92031a4bc649d
  First bad revision: 4b3932f9c4f5d9572da2f0232375474133191500
  Pushlog:
  https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b8b54a4990d7e778b82909a871e92031a4bc649d&tochange=4b3932f9c4f5d9572da2f0232375474133191500

Notes

• Attached a screenshot.
• I cannot reproduce the issue on macOS 10.14 or 10.15 with the same machine and external monitor.

Comment 1

[Haik Aftandilian [:haik]]

Harry, could you take a look? The regression range has a couple of appearance-related patches.

Comment 2

[Harry Twyford]

Before bug 1703892, we used the light NSColor.selectedTextBackgroundColor and simply halved the alpha channel to make it easier to discern white text underneath. After Bug 1700294, which added support for using different selection colors in dark and light modes, bug 1703892 stopped halving the alpha channel in dark mode because we could use the appropriate dark NSColor.selectedTextBackgroundColor. This doesn't work on 10.13 because 10.13 doesn't support dark mode.

We decide whether or not to halve alpha based on the ColorScheme value we pass to nsLookAndFeel::ProcessSelectionBackground. We should stop passing ColorScheme::Dark when the system does not support dark mode, like on 10.13.

Comment 3

[Harry Twyford]

Attachment: Bug 1725146 - Force ColorScheme::Light in NativeGetColor on macOS < 10.14. r?#mac-reviewers!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b69c43f3881c
Force ColorScheme::Light in NativeGetColor on macOS < 10.14. r=mac-reviewers,mstange

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b69c43f3881c

Comment 6

[Alexandru Trif, Desktop QA [:atrif]]

Verified fixed with Firefox 93.0a1 (20210824094724) on macOS 10.12 and 10.13. The text is now readable inside the address bar.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta and ESR91 approval when you get a chance.

Comment 8

[Harry Twyford]

Comment on attachment 9236515 [details]
Bug 1725146 - Force ColorScheme::Light in NativeGetColor on macOS < 10.14. r?#mac-reviewers!

Beta/Release Uplift Approval Request

• User impact if declined: Highlighted text in the Urlbar will have low contrast.
• 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 comment 0. It should be tested on both macOS <= 10.13 and >= 10.14.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small patch with well-understood effects. Tested in review by mstange on a machine running macOS 10.13.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a fairly serious UI regression and shouldn't sit for a year in ESR.
• User impact if declined: See above.
• Fix Landed on Version: 93
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small patch with well-understood effects.
• String or UUID changes made by this patch:

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9236515 [details]
Bug 1725146 - Force ColorScheme::Light in NativeGetColor on macOS < 10.14. r?#mac-reviewers!

Approved for 92.0b9 and 91.1esr.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/b2a5a197e0a1

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/47c210278de4

Comment 12

[Alexandru Trif, Desktop QA [:atrif]]

Verified fixed 91.1.0esr (20210825223222), 92.0b9 (20210825224239) from comment 10 and comment 11 on macOS 10.13 and macOS 10.12. The text inside the address bar is visible when selected while using Firefox dark theme.