Paul Silaghi, QA [:pauly] Reporter
	Description • 8 years ago

47.0a1 (2016-01-26) Win 7
Now I'm unable to use the keyboard for checking the "Report errors like this etc" option on https://wrong.host.badssl.com/. We should correct that.

	:Gijs (he/him) Assignee
	Comment 1 • 8 years ago

Seems like bug 1136645 got addressed in bug 1165973, but left out Linux, so now it has no focus styling for checkboxes, radio buttons and menulists.

All of the styling is broken for HTML input type="checkbox" because we hide the actual checkbox.

	:Gijs (he/him) Assignee
	Comment 2 • 8 years ago

Review commit: https://reviewboard.mozilla.org/r/33105/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33105/

	:Gijs (he/him) Assignee
	Comment 3 • 8 years ago

Note that the patch here uses a focus box around the checkbox instead of the label. Because of the HTML structure, it's not possible to do one only around the label (which is what we do for XUL checkboxes). This is unfortunate, but not a showstopper, I believe.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 4 • 8 years ago

Why not just change the HTML structure?

	:Gijs (he/him) Assignee
	Comment 5 • 8 years ago

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> Why not just change the HTML structure?

Because it's currently:

<input type="checkbox"/>
<label>Text to label checkbox</label>

and while we could add a container inside the label (like a span) to take care of this issue, that feels like not great markup, either. Plus we would need to update all the in-content pages that use checkboxes like this (not just about:certerror and about:neterror, but also including about:tabcrashed, about:debugging, and maybe about:networking if/when that uses in-content theming, and we'd need to remember to use a similar technique for any other future in-content HTML checkboxes...).

	:Gijs (he/him) Assignee
	Comment 6 • 8 years ago

(In reply to :Gijs Kruitbosch from comment #3)
> Note that the patch here uses a focus box around the checkbox instead of the
> label. Because of the HTML structure, it's not possible to do one only
> around the label (which is what we do for XUL checkboxes). This is
> unfortunate, but not a showstopper, I believe.

Also, this is only a compat issue with the XUL ones for Windows and Linux. On OS X the patch already produces the expected behaviour.

	:Gijs (he/him) Assignee
	Comment 7 • 8 years ago

(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to :Gijs Kruitbosch from comment #3)
> > Note that the patch here uses a focus box around the checkbox instead of the
> > label. Because of the HTML structure, it's not possible to do one only
> > around the label (which is what we do for XUL checkboxes). This is
> > unfortunate, but not a showstopper, I believe.
> 
> Also, this is only a compat issue with the XUL ones for Windows and Linux.
> On OS X the patch already produces the expected behaviour.

... that is, there we expect the focusring to be around the checkbox, not the label, and that is what happens. I believe the distinction is due to platform conventions.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 8 • 8 years ago

Comment on attachment 8714483 [details]
MozReview Request: Bug 1243353 - allow HTML in-content checkboxes to have focus and style them appropriately, r?jaws

https://reviewboard.mozilla.org/r/33105/#review30339

LGTM

	Pulsebot
	Comment 9 • 8 years ago

https://hg.mozilla.org/integration/fx-team/rev/4b0f903bdb28

	Phil Ringnalda (:philor)
	Comment 10 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/4b0f903bdb28

	Paul Silaghi, QA [:pauly] Reporter
	Comment 11 • 8 years ago

47.0a1 (2016-02-08) Win 7
Note that I can only check the option using the SPACE key, but not the ENTER key.

	:Gijs (he/him) Assignee
	Comment 12 • 8 years ago

(In reply to Paul Silaghi, QA [:pauly] from comment #11)
> 47.0a1 (2016-02-08) Win 7
> Note that I can only check the option using the SPACE key, but not the ENTER
> key.

This is normal behaviour for checkboxes, I believe. If we want to change that it would be a separate, orthogonal bug.

Needinfo for myself to uplift this to aurora with a=css-image-only later.

	:Gijs (he/him) Assignee
	Comment 13 • 8 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/a02e509588b0

	Paul Silaghi, QA [:pauly] Reporter
	Comment 14 • 8 years ago

Verified fixed Fx 46.0a2 (2016-02-12) Win 7