1705330 - [Proton] The focus ring on the Cancel/Save Changes buttons are cut off in the about:preferences modals

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P2)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: Dark OS theme

Affected Versions:
Nightly 89.0a1

Tested On:
Windows 10
Windows 10 ARM Yoga
MacOS 10.15 and MacOS 11 M1

Steps to reproduce:

1. Go to about:preferences#privacy
2. Open any model from the Permissions section, ex: Autoplay, Notifications etc
3. Navigate to the "Cancel" and "Save Changes" button using TAB navigation
4. Observe the focus ring for the mentioned buttons

Expected Result:
The focus ring should be properly displayed.

Actual Result:
The focus rings for the "Cancel/Save Changes" buttons are cut off

Notes:
This is reproducible only for the new focus rings introduced with Proton.

Comment 1

[Timea Cernea [:tbabos][inactive]]

Attachment: White OS theme

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1705330 - fix focus ring cut-off in preferences dialogs, r?jaws

These used to get 20px spacing from the browser element (frame). Now
they get 10px from the frame, and 6px padding on the root element, which
suffices to show these focus rings. I didn't move it all into the inner
doc because when things scroll, having some space between the doc and the
main prefs page seemed useful. I made the spacing smaller because that's
(ie 16px) the spec for proton modals and I figured we might as well make it
more consistent than it is right now.

As a bonus, I tested with the colours dialog and noticed that the dropdown
for "use document colors" (aka HCM) was much too tightly spaced vs. the
surrounding elements, so I added (again, proton-spec-based) 4px top +
bottom margin to let everything breathe a bit more.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4c7a1b5286a
fix focus ring cut-off in preferences dialogs, r=jaws,preferences-reviewers

Comment 4

[Cristian Brindusan [:cbrindusan]]

Backed out 3 changesets (Bug 1704113, Bug 1705330, Bug 1708148) for causing bc failures in browser_username_select_dialog.js and mochitest failures in test_modal_select.html.
https://hg.mozilla.org/integration/autoland/rev/3d2225b699c12f33140428b4be4bf8362c370df7

bc failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=30873124f2694ad0d98541fd68ca97db73b74856&selectedTaskRun=IAEk7U_iRwKWmtzhkjep1Q.0
Failure log:
https://treeherder.mozilla.org/logviewer?job_id=338103236&repo=autoland&lineNumber=3352

mochitest failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=30873124f2694ad0d98541fd68ca97db73b74856&selectedTaskRun=dmrgwgWdRCaH5ICCrOoWnA.0
Failure log:
https://treeherder.mozilla.org/logviewer?job_id=338103247&repo=autoland&lineNumber=2239

Comment 5

[:Gijs (he/him)]

(In reply to Cristian Brindusan [:cbrindusan] from comment #4)

Backed out 3 changesets (Bug 1704113, Bug 1705330, Bug 1708148) for causing bc failures in browser_username_select_dialog.js and mochitest failures in test_modal_select.html.

These are all related to bug 1708148, so I'll reland the 2 other bugs and investigate the failures in bug 1708148.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bbcba2cba6c
fix focus ring cut-off in preferences dialogs, r=jaws,preferences-reviewers

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/9bbcba2cba6c

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 9219069 [details]
Bug 1705330 - fix focus ring cut-off in preferences dialogs, r?jaws

Beta/Release Uplift Approval Request

• User impact if declined: Required for MR1 / Proton
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small, CSS-only patch that only affects dialogs.
• String changes made/needed: None.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 9219069 [details]
Bug 1705330 - fix focus ring cut-off in preferences dialogs, r?jaws

Actually, going to drop the uplift request. about:preferences was out of scope for 89 anyways.

Comment 10

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Verified in latest nightly build 90.0a1 (2021-05-13) (64-bit) Issue is no longer reproducible on win 10 pro , ubuntu 20.04, and mac0s 11.4 beta
Updating status accordingly.
Best regards,
Clara