1869442 - Tweak Report Broken Site CSS to improve focus feedback

Status: RESOLVED FIXED

(Firefox :: General, enhancement)

Description

[Thomas Wisniewski [:twisniewski]]

As per Itiel's suggestion, it may be best to alter this line (menulist:has(+ input:invalid)) to:

menulist:has(+ input:invalid):not(:focus-visible)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Why not using an html:select instead? I don't see why that wouldn't work and would avoid the ugly input hack.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Also :has is not unconditionally enabled in chrome code so if we want to use it we probably want to do that

Comment 3

[Thomas Wisniewski [:twisniewski]]

Why not using an html:select instead?

Good idea. I can try changing to a select soon. I remember I started with a select and changed to a menulist for some reason; possibly on request from Gijs or mossop, or possibly just because something wasn't working very well.

Also :has is not unconditionally enabled in chrome code so if we want to use it we probably want to do that

Good to know! Is there a blocker for why not? I'm already shipping that rule in 121/122, so it's a shame I didn't know about that until now (sorry) :S

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Thomas Wisniewski [:twisniewski] from comment #3)

Good to know! Is there a blocker for why not? I'm already shipping that rule in 121/122, so it's a shame I didn't know about that until now (sorry) :S

No, it's just a bit more work to make the preference not affect chrome stylesheets so we generally don't do it unless we need to.

Comment 5

[Thomas Wisniewski [:twisniewski]]

I just took another look at using html:select instead of menulist, and it will be a bit of a mess. The panel tab-key handling JS doesn't seem to recognize selects, so I can't tab to it in the UI. The styling is also not the same/branded for selects as it is for menulist. So I suspect it will be even more work to use select, sadly, and I think it's best for us to do that in a follow-up patch.

For now I'll just tweak the CSS for the focus behavior, since even if the red outline doesn't appear (because :has is disabled), then there will be a text message displayed under the input.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

(In reply to Thomas Wisniewski [:twisniewski] from comment #5)

I just took another look at using html:select instead of menulist, and it will be a bit of a mess. The panel tab-key handling JS doesn't seem to recognize selects, so I can't tab to it in the UI. The styling is also not the same/branded for selects as it is for menulist. So I suspect it will be even more work to use select, sadly, and I think it's best for us to do that in a follow-up patch.

That seems very fixable tho. Can you make sure there's at least a follow-up on file?

Comment 7

[Thomas Wisniewski [:twisniewski]]

That seems very fixable tho. Can you make sure there's at least a follow-up on file?

Yes. I just filed bug 1869823.

Comment 8

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1869442 - improve focus ring visibility on Report Broken Site inputs; r?emilio,itiel

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3aae77486d1
improve focus ring visibility on Report Broken Site inputs; r=emilio,desktop-theme-reviewers

Comment 10

[Noemi Erli[:noemi_erli]]

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