Get rid of some <label for> misuse in browser_options.html

Get rid of some <label for> misuse in browser_options.html

Two main cases:
1. <label> not wrapping input + span
2. for accessibility -- for=<selector-id> should instead use aria-labelledby

BUG=409938
Committed: https://crrev.com/6d0a8336e1cf1756af2269b7b1b517eb11cec857
Cr-Commit-Position: refs/heads/master@{#293215}

[Evan Stade, 2014-09-03 00:41:34]

estade@chromium.org changed reviewers:
+ arv@chromium.org

[Evan Stade, 2014-09-03 00:41:34]

at some point we should probably make a presubmit that chokes on <label for= >

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/controlled_setting.css (left):

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/controlled_setting.css:18: -webkit-user-select:
none;
I added this in a recent cl, but it should be moved to the .checkbox rule

[arv (Not doing code reviews), 2014-09-03 01:10:40]

LGTM

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.html:134: <span
id="default-search-engine-label"
This is a change in behavior. With the old markup you could click on the <label>
and the select would get focused.

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.html:223: <span
id="timezone-value-label" class="option-name"
same here

[Evan Stade, 2014-09-03 01:15:10]

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.html:134: <span
id="default-search-engine-label"
On 2014/09/03 01:10:40, arv wrote:
> This is a change in behavior. With the old markup you could click on the
<label>
> and the select would get focused.

yea, but I don't think that was intentional or desirable. I don't think "click
with mouse, then press spacebar" is a common pattern. Also, while you're
clicking, the <select> looks pressed, which is glitchy.

I looked up when this was added, and it was solely for the purposes of a11y.

[arv (Not doing code reviews), 2014-09-03 01:18:57]

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.html:134: <span
id="default-search-engine-label"
On 2014/09/03 01:15:10, Evan Stade wrote:
> On 2014/09/03 01:10:40, arv wrote:
> > This is a change in behavior. With the old markup you could click on the
> <label>
> > and the select would get focused.
> 
> yea, but I don't think that was intentional or desirable. I don't think "click
> with mouse, then press spacebar" is a common pattern. Also, while you're
> clicking, the <select> looks pressed, which is glitchy.
> 
> I looked up when this was added, and it was solely for the purposes of a11y.

Agreed. I think this is change for the better.

[Evan Stade, 2014-09-03 01:24:54]

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.html:134: <span
id="default-search-engine-label"
On 2014/09/03 01:18:57, arv wrote:
> On 2014/09/03 01:15:10, Evan Stade wrote:
> > On 2014/09/03 01:10:40, arv wrote:
> > > This is a change in behavior. With the old markup you could click on the
> > <label>
> > > and the select would get focused.
> > 
> > yea, but I don't think that was intentional or desirable. I don't think
"click
> > with mouse, then press spacebar" is a common pattern. Also, while you're
> > clicking, the <select> looks pressed, which is glitchy.
> > 
> > I looked up when this was added, and it was solely for the purposes of a11y.
> 
> Agreed. I think this is change for the better.

cool. Thanks for the speedy review.

[Evan Stade, 2014-09-03 16:58:45]

On 2014/09/03 01:24:54, Evan Stade wrote:
>
https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
> File chrome/browser/resources/options/browser_options.html (right):
> 
>
https://codereview.chromium.org/536703002/diff/40001/chrome/browser/resources...
> chrome/browser/resources/options/browser_options.html:134: <span
> id="default-search-engine-label"
> On 2014/09/03 01:18:57, arv wrote:
> > On 2014/09/03 01:15:10, Evan Stade wrote:
> > > On 2014/09/03 01:10:40, arv wrote:
> > > > This is a change in behavior. With the old markup you could click on the
> > > <label>
> > > > and the select would get focused.
> > > 
> > > yea, but I don't think that was intentional or desirable. I don't think
> "click
> > > with mouse, then press spacebar" is a common pattern. Also, while you're
> > > clicking, the <select> looks pressed, which is glitchy.
> > > 
> > > I looked up when this was added, and it was solely for the purposes of
a11y.
> > 
> > Agreed. I think this is change for the better.
> 
> cool. Thanks for the speedy review.

p.s., I just found the a11y audit extension and ran it, and it still passes the
"Controls and media
elements should have labels" test.

[Evan Stade, 2014-09-03 16:58:54]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2014-09-03 17:00:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/estade@chromium.org/536703002/40001

[commit-bot: I haz the power, 2014-09-03 18:11:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-09-03 18:49:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-03 18:49:31]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2014-09-03 19:52:02]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2014-09-03 19:53:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/estade@chromium.org/536703002/60001

[commit-bot: I haz the power, 2014-09-03 20:56:54]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2014-09-03 23:31:28]

Committed patchset #4 (id:60001) as 10f5508414e777f4076979b0bc3b2adfed2611b9

[commit-bot: I haz the power, 2014-09-10 03:28:21]

Patchset 4 (id:??) landed as
https://crrev.com/6d0a8336e1cf1756af2269b7b1b517eb11cec857
Cr-Commit-Position: refs/heads/master@{#293215}