MD Settings: circumvent problems with shadow DOM and aria radio* roles

MD Settings: circumvent problems with shadow DOM and aria radio* roles

They intentionally don't work across shadow DOM boundaries:
https://bugs.chromium.org/p/chromium/issues/detail?id=394302

R=hcarmona@chromium.org
BUG=709579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

[Dan Beam, 2017-04-11 06:52:15]

Description was changed from

==========
MD Settings: circumvent problems with shadow DOM and aria radio* roles

They intentionally don't work across shadow DOM boundaries:
https://bugs.chromium.org/p/chromium/issues/detail?id=394302

R=hcarmona@chromium.org
BUG=709579
==========

to

==========
MD Settings: circumvent problems with shadow DOM and aria radio* roles

They intentionally don't work across shadow DOM boundaries:
https://bugs.chromium.org/p/chromium/issues/detail?id=394302

R=hcarmona@chromium.org
BUG=709579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-11 06:55:25]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[Dan Beam, 2017-04-11 06:55:35]

Description was changed from

==========
MD Settings: circumvent problems with shadow DOM and aria radio* roles

They intentionally don't work across shadow DOM boundaries:
https://bugs.chromium.org/p/chromium/issues/detail?id=394302

R=hcarmona@chromium.org
BUG=709579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: circumvent problems with shadow DOM and aria radio* roles

They intentionally don't work across shadow DOM boundaries:
https://bugs.chromium.org/p/chromium/issues/detail?id=394302

R=hcarmona@chromium.org
BUG=709579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2017-04-11 06:55:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2017-04-11 06:55:50]

/cc dmazzoni@ to see if he knows what's going on here

[commit-bot: I haz the power, 2017-04-11 07:51:04]

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

[commit-bot: I haz the power, 2017-04-11 07:51:05]

Dry run: This issue passed the CQ dry run.

[hcarmona, 2017-04-11 17:47:25]

I feel like we keep moving properties from paper-radio-button to
controlled-radio-button. Should we look at maybe changing this so we can have an
extends/is relationship?

ex: <paper-radio-button is="controlled-radio-button">

https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/controls/controlled_radio_button.js
(right):

https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/controls/controlled_radio_button.js:52:
this.removeAttribute('aria-label');
There's only one case where label isn't set.

https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/appear...

After this change, the radio and input are unlabelled.

https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/controls/settings_radio_group.js (right):

https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/controls/settings_radio_group.js:36: role:
'radiogroup',
Is this needed? The radio buttons are already children of paper-radio-group

[Dan Beam, 2017-04-11 19:06:24]

On 2017/04/11 17:47:25, hcarmona wrote:
> I feel like we keep moving properties from paper-radio-button to
> controlled-radio-button. Should we look at maybe changing this so we can have
an
> extends/is relationship?
> 
> ex: <paper-radio-button is="controlled-radio-button">

you can only customize built-in elements:
https://html.spec.whatwg.org/multipage/scripting.html#attr-is

> 
>
https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
> File chrome/browser/resources/settings/controls/controlled_radio_button.js
> (right):
> 
>
https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
> chrome/browser/resources/settings/controls/controlled_radio_button.js:52:
> this.removeAttribute('aria-label');
> There's only one case where label isn't set.
> 
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/appear...
> 
> After this change, the radio and input are unlabelled.
> 
>
https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
> File chrome/browser/resources/settings/controls/settings_radio_group.js
(right):
> 
>
https://codereview.chromium.org/2815593002/diff/1/chrome/browser/resources/se...
> chrome/browser/resources/settings/controls/settings_radio_group.js:36: role:
> 'radiogroup',
> Is this needed? The radio buttons are already children of paper-radio-group

[Dan Beam, 2017-04-11 19:27:25]

braindump time!

I've been talking a little bit with dmazzoni@ about this off thread.

dmazzoni@ looked at the a11y tree and found this of our current code on trunk:

radioGroup
  unknown focusable
    radioButton focusable
  unknown focusable
    radioButton focusable

unknown is the controlled-radio-button.  I don't know why they're focusable.

we both thought "we should use role='presentation/none'" which didn't work.

in googling last night, I found the bug linked to from the CL desc about how
role="radio" shouldn't work across shadow DOM boundaries:
https://bugs.chromium.org/p/chromium/issues/detail?id=394302

so it seems like if wrap a

<custom-thing role="radiogroup">
  <custom-thing-button role="radio" value="1">
  <custom-thing-button role="radio" value="2">
  <custom-thing-button role="radio" value="3">
</custom-thing>

it will work because it doesn't actually cross shadow DOM boundaries, but only
light DOM.  this is probably how <paper-radio-group>/<paper-radio-button> works.

[hcarmona, 2017-04-11 20:50:14]

On 2017/04/11 19:27:25, Dan Beam wrote:
> braindump time!
> 
> I've been talking a little bit with dmazzoni@ about this off thread.
> 
> dmazzoni@ looked at the a11y tree and found this of our current code on trunk:
> 
> radioGroup
>   unknown focusable
>     radioButton focusable
>   unknown focusable
>     radioButton focusable
> 
> unknown is the controlled-radio-button.  I don't know why they're focusable.
> 
> we both thought "we should use role='presentation/none'" which didn't work.
> 
> in googling last night, I found the bug linked to from the CL desc about how
> role="radio" shouldn't work across shadow DOM boundaries:
> https://bugs.chromium.org/p/chromium/issues/detail?id=394302
> 
> so it seems like if wrap a
> 
> <custom-thing role="radiogroup">
>   <custom-thing-button role="radio" value="1">
>   <custom-thing-button role="radio" value="2">
>   <custom-thing-button role="radio" value="3">
> </custom-thing>
> 
> it will work because it doesn't actually cross shadow DOM boundaries, but only
> light DOM.  this is probably how <paper-radio-group>/<paper-radio-button>
works.

This structure makes sense:

radioGroup                - paper-radio-group
  unknown focusable       - controlled-radio-button's :host
    radioButton focusable - paper-radio-button

The issue is that the controlled-radio-button isn't a radio button.
We've been working around the radio role not going across the shadow dom
by pushing the properties that we need to the controlled-radio-button.

The paper-radio-group only updates its direct children, so we've pushed
focus and checked props from the paper radio to the controlled radio.
It looks like we need to add the role and label to the controlled radio
also. It's unfortunate that we can't extend custom elements.

[Dan Beam, 2017-04-12 16:09:55]

sorry for the radio silence on this yesterday

my current plan of attack is to override setAttribute() to forward along
properties like tabindex without actually seeing them on :host

[Dan Beam, 2017-04-13 00:16:21]

let's check out an alternative here:
https://codereview.chromium.org/2816883002/