Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0 Build ID: 20180301100139 [Affected versions]: Nightly 60.0a1 [Affected platforms]: Windows 7 x86, Windows 10 x64 [Steps to reproduce]: 1.Launch Nightly 60.0a1 with a new profile 2.Go to about:preferences#privacy and click "Clear Data" button from Cookies and Dite Data section 3.Press Tab key to navigate through Clear Data options ("Cookies and Site Data" and "Cached Web Content") [Expected result]: The options should be selected and highlighted [Actual result]: The options aren't selected and highlighted but a comma appear next to option checkbox

Comment on attachment 8968534 [details] Bug 1442542 - Make checkboxes in the clear site data dialog use a label attribute. https://reviewboard.mozilla.org/r/237220/#review243298 ::: browser/themes/windows/preferences/preferences.css:82 (Diff revision 1) > } > + > +/* Clear Site Data Dialog */ > + > +/* We're not using regular checkbox labels (because we want to > + * have a custom hint below the label), which is messing up This also breaks clicking the label to toggle the checkbox. Why are we center-aligning the checkbox with both labels rather than only with the primary one? ::: browser/themes/windows/preferences/preferences.css:83 (Diff revision 1) > + > +/* Clear Site Data Dialog */ > + > +/* We're not using regular checkbox labels (because we want to > + * have a custom hint below the label), which is messing up > + * focus rings on Windows for some reason, so we'll just draw our own. */ The reason is that we draw the focus ring around .checkbox-label-box, as you already seem to have realized. It's a problem on Linux too. ::: browser/themes/windows/preferences/preferences.css:84 (Diff revision 1) > +/* Clear Site Data Dialog */ > + > +/* We're not using regular checkbox labels (because we want to > + * have a custom hint below the label), which is messing up > + * focus rings on Windows for some reason, so we'll just draw our own. */ > +#ClearSiteDataDialog checkbox:focus > .checkbox-check { Should be :-moz-focusring (except that I don't think this is the right solution in the first place)

(In reply to Dão Gottwald [::dao] from comment #2) > Comment on attachment 8968534 [details] > Bug 1442542 - Fix focusring for checkboxes in the clear site data dialog. > > https://reviewboard.mozilla.org/r/237220/#review243298 > > ::: browser/themes/windows/preferences/preferences.css:82 > (Diff revision 1) > > } > > + > > +/* Clear Site Data Dialog */ > > + > > +/* We're not using regular checkbox labels (because we want to > > + * have a custom hint below the label), which is messing up > > This also breaks clicking the label to toggle the checkbox. You're right, we should file a bug about that, I'm pretty sure the control attribute should take care of it... > Why are we > center-aligning the checkbox with both labels rather than only with the > primary one? Because that's what the design spec says. I don't think it's an unreasonable request to allow checkboxes to be controlled by label elements that were not set via label="". It works in HTML and it should work in XUL.

(In reply to Johann Hofmann [:johannh] from comment #3) > > This also breaks clicking the label to toggle the checkbox. > > You're right, we should file a bug about that, I'm pretty sure the control > attribute should take care of it... Well, we can file a bug on it, but we can't necessarily expect it to be fixed. XUL is on life support. > > Why are we > > center-aligning the checkbox with both labels rather than only with the > > primary one? > > Because that's what the design spec says. That's circular reasoning. Why does the spec say so? Can it be revisited?

I agree with the spec that it looks better with both aligned, but I guess there's a point in that XUL won't ever support this. *sigh*

(In reply to Johann Hofmann [:johannh] from comment #5) > I agree with the spec that it looks better with both aligned, If you take into account accessibility, namely that users might expect the label to be clickable, then I think it makes quite a bit more sense to align the primary label with the checkbox. The secondary label just provides context and shouldn't be clickable.

Comment on attachment 8968534 [details] Bug 1442542 - Make checkboxes in the clear site data dialog use a label attribute. https://reviewboard.mozilla.org/r/237220/#review243700 The FTL looks good. Also CCed Zibi in case he wants to take a look at the code (not sure if you want to actually flag him)

Can you add this to your patch? It allows us to migrate strings from old .properties and DTDs. These recipes live in https://hg.mozilla.org/mozilla-unified/file/central/python/l10n/fluent_migrations

Yay! That looks awesome! I did plan to do a bit more about unit formatting via mozIntl, and the result will affect localization here because we'll end up with sth like: ``` key = Cookies and Site Data ({ UNIT($amount) }) ``` and: ``` document.l10n.setAttributes(element, "key", { amount: FluentUnit(amount, { unit }) }); ``` but that's a more sizable project to get to (because I didn't write UnitFormat in bug 1415813 yet ;)), so let's land it now :)

Comment on attachment 8968534 [details] Bug 1442542 - Make checkboxes in the clear site data dialog use a label attribute. https://reviewboard.mozilla.org/r/237220/#review243890 ::: browser/components/preferences/clearSiteData.xul:25 (Diff revision 2) > <script type="application/javascript" src="chrome://global/content/l10n.js"></script> > > <script src="chrome://browser/content/preferences/clearSiteData.js"/> > > <stringbundle id="bundlePreferences" > src="chrome://browser/locale/preferences/preferences.properties"/> Can you also remove this <stringbundle>?

Comment on attachment 8968534 [details] Bug 1442542 - Make checkboxes in the clear site data dialog use a label attribute. https://reviewboard.mozilla.org/r/237220/#review244156 ::: browser/components/preferences/clearSiteData.xul:36 (Diff revision 3) > - control="clearSiteData" > - data-l10n-id="clear-site-data-cookies" /> > - <description class="option-description" data-l10n-id="clear-site-data-cookies-info" /> > - </vbox> > + </vbox> > - </hbox> > - <hbox class="option"> > + <vbox class="option"> > + <checkbox data-l10n-id="clear-site-data-cache-empty" id="clearCache" checked="true" /> nit: remove space before /> while you're touching this

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 16499ea8a9d0bd737a93d01bd118f8380d662e03 -d b758bc75b054: rebasing 459524:16499ea8a9d0 "Bug 1442542 - Make checkboxes in the clear site data dialog use a label attribute. r=dao,flod" (tip) merging browser/locales/jar.mn warning: conflicts while merging browser/locales/jar.mn! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Pushed by jhofmann@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2ddb032c4520 Make checkboxes in the clear site data dialog use a label attribute. r=dao,flod

Retested this issue on Windows 10 x64 and Windows 7 x64 using the latest Nightly 61.0a1(2018-04-25). Using the Tab key,the options are selected but the checkboxes aren't highlighted. On mouse hover, the options are selected and the checkboxes are highlighted. ( please see the screenshot) Johann, should I open a new bug for the highlight behaviour?

(In reply to roxana.leitan@softvision.ro from comment #21) > Retested this issue on Windows 10 x64 and Windows 7 x64 using the latest > Nightly 61.0a1(2018-04-25). > Using the Tab key,the options are selected but the checkboxes aren't > highlighted. > On mouse hover, the options are selected and the checkboxes are highlighted. > ( please see the screenshot) > > Johann, should I open a new bug for the highlight behaviour? No, I don't think that's unexpected :)

Based on comment 21 and comment 22 I will mark this bug Verified fixed on Fx61.