1516767 - about:preferences buttons have invisible border in high contrast mode

Status: VERIFIED FIXED

(Toolkit :: Themes, defect)

Comment 1

[Tim Nguyen :ntim]

Attachment: buttoncss.PNG

Comment 2

[Justin Dolske [:Dolske]]

Over to Gijs (at least as a first owner). Would like fix in the next week, as the last two betas GTB on Monday and Thursday.

Comment 3

[:Gijs (he/him)]

This is because the patch in bug 1511394 sets the border to 0/none for all buttons etc. An earlier iteration of the patch in bug 1511394 did have

border: 1px solid transparent;

But Dão commented that this would change the border on focus. I don't really understand why it would; I don't see any changes on focus with/without the border being set like that. What it does do is set a 1px border which makes the button visible in HCM because of the "forced" foreground/background colours.

This won't change any visible heights because we use border-box sizing for these elements in XUL ( https://searchfox.org/mozilla-central/rev/7adb490485eff0783071a3e132005bceeb337461/toolkit/content/minimal-xul.css#27 ) and HTML ( https://searchfox.org/mozilla-central/rev/7adb490485eff0783071a3e132005bceeb337461/layout/style/res/forms.css#606 ) and we set a min-height of 32px.

For cases with large windows font sizes, I'd expect that if anything, this would make the buttons look slightly better because there'd be more space between the text and the top/bottom of the button.

At least in the prefs, I don't see any width changes either though I have to admit I'm not 100% sure why not.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1516767 - adjust button border, outline and box-shadow to be compatible with High-Contrast Mode and less glitchy, r?dao,jaws

Comment 5

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #3)

This is because the patch in bug 1511394 sets the border to 0/none for all buttons etc. An earlier iteration of the patch in bug 1511394 did have

border: 1px solid transparent;

But Dão commented that this would change the border on focus.

Specifically, here . Dão, if you want to clarify what you meant or if there should be a different fix here, do let me know.

Comment 6

[Tim Nguyen :ntim]

Attachment: Broken focusring with patch applied

Comment 7

[Tim Nguyen :ntim]

Attachment: Expected focusring

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbc1cb5a84a9
adjust button border, outline and box-shadow to be compatible with High-Contrast Mode and less glitchy, r=jaws

Comment 9

[Narcis Beleuzu [:NarcisB]]

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

Comment 10

[:Gijs (he/him)]

Comment on attachment 9035953 [details]
Bug 1516767 - adjust button border, outline and box-shadow to be compatible with High-Contrast Mode and less glitchy, r?dao,jaws

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1511394

User impact if declined: Can't identify buttons when using Firefox in high-contrast mode

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: Check visual appearance of buttons, menulists, and HTML select dropdowns in in-content pages that use common.inc.css (about:preferences (XUL buttons + menulists), about:telemetry (HTML selects, by going to 'scalars' and focusing the 'ping type' dropdown on the top right), about:tabcrashed (HTML buttons)), in "normal" themes across OSes, and Windows High Contrast Mode.

List of other uplifts needed: n/a

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Specifically adjusts these focus styles, CSS-only patch, so fairly low risk in that sense, but of course it's now fairly late in beta...

String changes made/needed: No

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9035953 [details]
Bug 1516767 - adjust button border, outline and box-shadow to be compatible with High-Contrast Mode and less glitchy, r?dao,jaws

This doesn't graft cleanly because bug 1512332 isn't on Beta (and bug 1437641 to a lesser extent). Please create a branch patch and re-nominate for uplift (you can skip the form the second time around :)...).

Comment 12

[Tim Nguyen :ntim]

I've just added a note on high contrast on https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/CSS_Guidelines#High_contrast_mode in hopes that it will prevent future regressions :)

Comment 13

[:Gijs (he/him)]

Attachment: Patch for beta

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9036312 [details] [diff] [review]
Patch for beta

[Triage Comment]
Theming improvements for users with High Contrast mode enabled. Approved for 65.0b11.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3fa15331a220

Comment 16

[Maria Berlinger [:mberlinger], Services QA]

I have managed to reproduce this issue on an affected Firefox 65.0b10 build using Windows 10 x64.

This issue is verified fixed using Firefox 65.0b11 and Firefox 66.0a1(20190114215201) on the following OSes: Windows 10 x64, Ubuntu 16.04 x64, Windows 7 x64, Windows 8.1 x64, macOS 10.12.6.

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8a9ead048034