Fixing mapping conflicts for required html attribute and aria-required

Fixing mapping conflicts for required html attribute and aria-required

When WAI-ARIA states and properties correspond to host language features that
have the same implicit WAI-ARIA semantic, it can be problematic if the values
become out of sync. For example, the HTML required attribute and the aria-required
attribute could have conflicting values. Therefore to prevent providing
conflicting states and properties to assistive technologies, host languages will
explicitly declare where the use of WAI-ARIA attributes on a host language
element conflict with native attributes for that element. When a host language
declares a WAI-ARIA attribute to be in direct semantic conflict with a native
attribute for a given element, user agents MUST ignore the WAI-ARIA attribute
and instead use the host language attribute with the same implicit semantic."

BUG=421337
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187431

[shreeramk, 2014-10-07 17:35:48]

shreeram.k@samsung.com changed reviewers:
+ aboxhall@chromium.org, dmazzoni@chromium.org, mkwst@chromium.org

[shreeramk, 2014-10-07 17:35:49]

Please review.

Thanks!

[Mike West, 2014-10-07 19:37:04]

On 2014/10/07 17:35:49, shreeram.k wrote:
> Please review.
> 
> Thanks!

Hrm. It's not entirely clear to me what the impact of this change will be:
<input required="true"> would always be required, regardless of the ARIA
attribute? And <input required="false"> never? This basically has the result of
making the ARIA attribute meaningless on form elements; it's not clear that
that's web compatible, though the CL description makes a reasonable case for it.

Do we not have tests for this code?

[shreeramk, 2014-10-08 02:57:16]

On 2014/10/07 19:37:04, Mike West wrote:
> On 2014/10/07 17:35:49, shreeram.k wrote:
> > Please review.
> > 
> > Thanks!
> 
> Hrm. It's not entirely clear to me what the impact of this change will be:
> <input required="true"> would always be required, regardless of the ARIA
> attribute? And <input required="false"> never? This basically has the result
of
> making the ARIA attribute meaningless on form elements; it's not clear that
> that's web compatible, though the CL description makes a reasonable case for
it.
> 
> Do we not have tests for this code?

Consider following cases

Ideally we should test at least four different scenarios:

1. <div tabIndex=0 role="textbox" aria-required="true">
2. <input type="text" required>
3. <input type="text" aria-required="true">

#1 should have IA2_STATE_READONLY
#2 should also have IA2_STATE_READONLY
#3 shouldn't have IA2_STATE_READONLY

[dmazzoni, 2014-10-08 07:00:13]

Please file a bug and reference the bug number in the change.

[dmazzoni, 2014-10-08 07:02:26]

On 2014/10/07 19:37:04, Mike West wrote:
> Do we not have tests for this code?

I'm assuming Shreeram will follow up with a Chromium-side test.

[shreeramk, 2014-10-08 07:17:06]

On 2014/10/08 07:02:26, dmazzoni wrote:
> On 2014/10/07 19:37:04, Mike West wrote:
> > Do we not have tests for this code?
> 
> I'm assuming Shreeram will follow up with a Chromium-side test.

Bug filed and referenced in bug description.

[shreeramk, 2014-10-08 07:17:24]

On 2014/10/08 07:02:26, dmazzoni wrote:
> On 2014/10/07 19:37:04, Mike West wrote:
> > Do we not have tests for this code?
> 
> I'm assuming Shreeram will follow up with a Chromium-side test.

Yes I ll follow up with chromium side test.

[Mike West, 2014-10-08 08:29:49]

On 2014/10/08 02:57:16, shreeram.k wrote:
> 3. <input type="text" aria-required="true">

#1 and #2 make perfect sense, but I'm not entirely convinced on this case. Why
wouldn't it make sense to treat the input element as required if _either_ the
ARIA attribute or `required` attribute are set?

-mike

[dmazzoni, 2014-10-08 19:21:45]

On 2014/10/08 08:29:49, Mike West wrote:
> On 2014/10/08 02:57:16, shreeram.k wrote:
> > 3. <input type="text" aria-required="true">
> 
> #1 and #2 make perfect sense, but I'm not entirely convinced on this case. Why
> wouldn't it make sense to treat the input element as required if _either_ the
> ARIA attribute or `required` attribute are set?

The spec authors decided otherwise and we should follow that consensus or file a
W3C bug if we disagree. It's not quite so obvious in the case of 'required', but
I think their concern was that if you have <input type="checkbox"> that's
checked, you shouldn't be able to override it with aria-checked to make it
appear unchecked.

[shreeramk, 2014-10-10 05:37:56]

On 2014/10/08 19:21:45, dmazzoni wrote:
> On 2014/10/08 08:29:49, Mike West wrote:
> > On 2014/10/08 02:57:16, shreeram.k wrote:
> > > 3. <input type="text" aria-required="true">
> > 
> > #1 and #2 make perfect sense, but I'm not entirely convinced on this case.
Why
> > wouldn't it make sense to treat the input element as required if _either_
the
> > ARIA attribute or `required` attribute are set?
> 
> The spec authors decided otherwise and we should follow that consensus or file
a
> W3C bug if we disagree. It's not quite so obvious in the case of 'required',
but
> I think their concern was that if you have <input type="checkbox"> that's
> checked, you shouldn't be able to override it with aria-checked to make it
> appear unchecked.

What about readonly and aria-readonly?

I think this is also an important attribute that can conflict.

I observed that readonly and aria-readonly implementation is not correct in
code.


For example:

<html>
<body>
  <input aria-readonly="false" tabindex="0" value="Editable text">
  <input aria-readonly="true" tabindex="1" value="Uneditable text">
  <input readonly tabindex="1" value="Uneditable text">
</body>
</html>

The AX tree on win is
ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE
    IA2_ROLE_SECTION READONLY
        ROLE_SYSTEM_TEXT value='Editable text' FOCUSABLE IA2_STATE_EDITABLE
IA2_STATE_SELECTABLE_TEXT IA2_STATE_SINGLE_LINE
        ROLE_SYSTEM_TEXT value='Uneditable text' READONLY FOCUSABLE
IA2_STATE_EDITABLE IA2_STATE_SELECTABLE_TEXT IA2_STATE_SINGLE_LINE
        ROLE_SYSTEM_TEXT value='Uneditable text' FOCUSABLE IA2_STATE_EDITABLE
IA2_STATE_SELECTABLE_TEXT IA2_STATE_SINGLE_LINE

If you see, when aria-readonly="true" then both READONLY and IA2_STATE_EDITABLE
are getting exposed, which is totally wrong.

Also for html input attribute readonly is not respected in the AX tree, its
giving IA2_STATE_EDITABLE instead of READONLY.

[shreeramk, 2014-12-05 07:32:54]

https://codereview.chromium.org/631183002/diff/1/Source/core/accessibility/AX...
File Source/core/accessibility/AXNodeObject.cpp (right):

https://codereview.chromium.org/631183002/diff/1/Source/core/accessibility/AX...
Source/core/accessibility/AXNodeObject.cpp:753: if
(equalIgnoringCase(getAttribute(aria_requiredAttr), "true"))
Spec says http://w3c.github.io/aria/aria/aria.html#aria-required

"Unless an exactly equivalent native attribute is available, host languages
should allow authors to use the aria-required attribute on host language form
elements that require input or selection by the user."

As we know html "required" attribute exists, then I think, in that scenario we
should give first priority to required attribute and then to aria-required
attribute for form elements.

Also for form elements we should first check for html required attribute then
check for aria-required attribute.

[shreeramk, 2014-12-17 16:52:45]

On 2014/12/05 07:32:54, shreeramk wrote:
>
https://codereview.chromium.org/631183002/diff/1/Source/core/accessibility/AX...
> File Source/core/accessibility/AXNodeObject.cpp (right):
> 
>
https://codereview.chromium.org/631183002/diff/1/Source/core/accessibility/AX...
> Source/core/accessibility/AXNodeObject.cpp:753: if
> (equalIgnoringCase(getAttribute(aria_requiredAttr), "true"))
> Spec says http://w3c.github.io/aria/aria/aria.html#aria-required
> 
> "Unless an exactly equivalent native attribute is available, host languages
> should allow authors to use the aria-required attribute on host language form
> elements that require input or selection by the user."
> 
> As we know html "required" attribute exists, then I think, in that scenario we
> should give first priority to required attribute and then to aria-required
> attribute for form elements.
> 
> Also for form elements we should first check for html required attribute then
> check for aria-required attribute.

@dominic: PTAL(please refer my prev comment).
Thanks...

[dmazzoni, 2014-12-17 17:09:52]

lgtm

[shreeramk, 2014-12-18 04:03:32]

The CQ bit was checked by shreeram.k@samsung.com

[commit-bot: I haz the power, 2014-12-18 04:04:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/631183002/30001

[commit-bot: I haz the power, 2014-12-18 04:59:37]

Committed patchset #2 (id:30001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187431