Expose semantics of landmark and related roles for assitive technologies

Expose semantics of HTML5 native elements (landmark and related roles)
for assistive technologies on Windows. This changeset adds support for

- article (xml-roles:article)
- header (xml-roles:banner)
- aside (xml-roles:complementary)
- address (xml-roles:contentinfo)
- footer (xml-roles:contentinfo)
- main (xml-roles:main)
- nav (xml-roles:navigation)
- section (xml-roles:region)

Patch from Takeshi Kurosawa <taken.spc@gmail.com>;
BUG=253027
TEST=content\renderer\accessibility\accessibility_node_serializer.cc

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243522

[Takeshi Kurosawa, 2014-01-03 17:15:34]

https://codereview.chromium.org/104583006/ is a related blink changeset. Without
that, the landmark and related objects will be ignored and not exposed in many
cases.

[dmazzoni, 2014-01-03 18:16:54]

Thanks! Making this work sounds great, but it shouldn't be necessary to change
this source file - we already expose the "role" attribute in xml-roles using
special-case code in browser_accessibility_win.cc.

Try adding a new test to dump_accessibility_tree_browsertest.cc (add files to
content/test/data/accessibility). If that fails after your Blink change lands,
we should figure out why.

Specifically, look at heading.html and article.html in
content/test/data/accessibility for examples of tests that look at xml-roles on
Windows.

[Takeshi Kurosawa, 2014-01-04 12:02:13]

Thank you for your review.

On 2014/01/03 18:16:54, Dominic Mazzoni wrote:
> Thanks! Making this work sounds great, but it shouldn't be necessary to change
> this source file - we already expose the "role" attribute in xml-roles using
> special-case code in browser_accessibility_win.cc.

The purpose of this changeset is adding support for the HTML5 native elements
which don't have role attribute (e.g. <main></main> instead of <main
role="main"></main>). So I added WebAXRole to xml-roles value conversion code
(in addition to code for copying role attribute which we already have).
# By the way, blink and chromium seems not to have internal role corresponding
to role="form".

> Try adding a new test to dump_accessibility_tree_browsertest.cc (add files to
> content/test/data/accessibility). If that fails after your Blink change lands,
> we should figure out why.
> 
> Specifically, look at heading.html and article.html in
> content/test/data/accessibility for examples of tests that look at xml-roles
on
> Windows.

I added few tests. As I don't have mac and android development environments, I
added only -expected-win.txt files.

[dmazzoni, 2014-01-05 05:27:31]

On 2014/01/04 12:02:13, taken wrote:
> The purpose of this changeset is adding support for the HTML5 native elements
> which don't have role attribute (e.g. <main></main> instead of <main
> role="main"></main>). So I added WebAXRole to xml-roles value conversion code
> (in addition to code for copying role attribute which we already have).

Oh, I understand now, thanks. Let's rename ConvertRole to be more clear.

I didn't realize that an HTML element like <main> should actually expose "main"
in xml-roles.

Should we be doing this for other roles? Like, should a <button> expose an
xml-role of "button" and should <input type="range"> expose an xml-role of
"slider"? If not, why not?

> # By the way, blink and chromium seems not to have internal role corresponding
> to role="form".

Are you sure? I see blink::WebAXRoleForm, is it not working?

> I added few tests. As I don't have mac and android development environments, I
> added only -expected-win.txt files.

If the tests still pass that's fine, since the changes you're making only apply
on Windows (where there's an xml-role feature). I'll run try jobs now.

[dmazzoni, 2014-01-05 05:27:38]

https://codereview.chromium.org/98183011/diff/90001/content/renderer/accessib...
File content/renderer/accessibility/accessibility_node_serializer.cc (right):

https://codereview.chromium.org/98183011/diff/90001/content/renderer/accessib...
content/renderer/accessibility/accessibility_node_serializer.cc:127: std::string
ConvertRole(const blink::WebAXRole role) {
Let's call this GetEquivalentAriaRoleString.

https://codereview.chromium.org/98183011/diff/90001/content/renderer/accessib...
content/renderer/accessibility/accessibility_node_serializer.cc:307: if
(!role.empty()) {
Nit: the Chromium style is to not use { } braces when both the conditional and
the body are one line each.

https://codereview.chromium.org/98183011/diff/90001/content/test/data/accessi...
File content/test/data/accessibility/landmark-expected-win.txt (right):

https://codereview.chromium.org/98183011/diff/90001/content/test/data/accessi...
content/test/data/accessibility/landmark-expected-win.txt:14: IA2_ROLE_SECTION
xml-roles:appliation
typo

https://codereview.chromium.org/98183011/diff/90001/content/test/data/accessi...
content/test/data/accessibility/landmark-expected-win.txt:18: IA2_ROLE_SECTION
xml-roles:complemtary
typo

[Takeshi Kurosawa, 2014-01-05 23:19:54]

On 2014/01/05 05:27:31, Dominic Mazzoni wrote:
> On 2014/01/04 12:02:13, taken wrote:
> > The purpose of this changeset is adding support for the HTML5 native
elements
> > which don't have role attribute (e.g. <main></main> instead of <main
> > role="main"></main>). So I added WebAXRole to xml-roles value conversion
code
> > (in addition to code for copying role attribute which we already have).
> 
> Oh, I understand now, thanks. Let's rename ConvertRole to be more clear.
> 
> I didn't realize that an HTML element like <main> should actually expose
"main"
> in xml-roles.
> 
> Should we be doing this for other roles? Like, should a <button> expose an
> xml-role of "button" and should <input type="range"> expose an xml-role of
> "slider"? If not, why not?

In my understanding, xml-roles is used to distinguish html elements/aria roles
that not have exactly corresponding accessibility objects.
# sliders and links can be distinguished by their accessibility objects roles.

HTML to Platform Accessibility APIs Implementation Guide
http://rawgithub.com/w3c/html-api-map/master/index.html#html-element-to-acces...
 

> > # By the way, blink and chromium seems not to have internal role
corresponding
> > to role="form".
> 
> Are you sure? I see blink::WebAXRoleForm, is it not working?

I mean createARIARoleMap doesn't have "form" entry (role="form" is exposed as
xml-roles:form).
http://src.chromium.org/viewvc/blink/trunk/Source/core/accessibility/AXObject...

[dmazzoni, 2014-01-06 09:18:56]

You can get the Mac and Android expectations from the logs, e.g.:

http://build.chromium.org/p/tryserver.chromium/builders/mac/builds/113179/ste...

[dmazzoni, 2014-01-06 09:22:13]

On 2014/01/05 23:19:54, taken wrote:
> In my understanding, xml-roles is used to distinguish html elements/aria roles
> that not have exactly corresponding accessibility objects.
> 
> # sliders and links can be distinguished by their accessibility objects roles.

OK, but the browser still returns xml-role:button if you add a role="button" -
so in practice it always returns an xml-role if you specify an ARIA role. I just
wonder why it shouldn't always return a role.

> I mean createARIARoleMap doesn't have "form" entry (role="form" is exposed as
> xml-roles:form).
>
http://src.chromium.org/viewvc/blink/trunk/Source/core/accessibility/AXObject...

Oh, I see. Yes, that looks like an error, please feel free to send a patch (or
if you don't have time, file a bug).

Thanks for your contributions!

[Takeshi Kurosawa, 2014-01-06 14:24:00]

Thank you for your reviews and kindness.
I addressed your review comments (improved naming and fixed several typos)
tweaked description and added -expected-android.txt and -expected-mac.txt.

On 2014/01/06 09:22:13, Dominic Mazzoni wrote:
> On 2014/01/05 23:19:54, taken wrote:
> > In my understanding, xml-roles is used to distinguish html elements/aria
roles
> > that not have exactly corresponding accessibility objects.
> > 
> > # sliders and links can be distinguished by their accessibility objects
roles.
> 
> OK, but the browser still returns xml-role:button if you add a role="button" -
> so in practice it always returns an xml-role if you specify an ARIA role. I
just
> wonder why it shouldn't always return a role.

It seems historical reason. I guess it is OK to return xml-role for all elements
which have corresponding role values. However I think ATs won't use xml-role for
such elements.

http://asurkov.blogspot.jp/2011/11/firefox-11-for-at-developers.html
Mozilla returns xml-role:figure for figure elements to make ATs be able to
detect figure elements (there is no figure role in WAI-ARIA 1.0).

> > I mean createARIARoleMap doesn't have "form" entry (role="form" is exposed
as
> > xml-roles:form).
> >
>
http://src.chromium.org/viewvc/blink/trunk/Source/core/accessibility/AXObject...
> 
> Oh, I see. Yes, that looks like an error, please feel free to send a patch (or
> if you don't have time, file a bug).

I filed it.
https://code.google.com/p/chromium/issues/detail?id=331789

[dmazzoni, 2014-01-06 22:35:56]

lgtm

Great! I'll land this shortly if the tryjobs pass.

[Takeshi Kurosawa, 2014-01-07 13:52:00]

On 2014/01/06 22:35:56, Dominic Mazzoni wrote:
> lgtm
> 
> Great! I'll land this shortly if the tryjobs pass.

Sorry for Mac failures.
The reason of mac failures is probably lacking "@MAC-ALLOW:AXSubrole=*" of
landmark.html.
As content/ was refactored, I will update the patch set.

[dmazzoni, 2014-01-07 16:22:31]

On 2014/01/07 13:52:00, taken wrote:
> On 2014/01/06 22:35:56, Dominic Mazzoni wrote:
> > lgtm
> > 
> > Great! I'll land this shortly if the tryjobs pass.
> 
> Sorry for Mac failures.
> The reason of mac failures is probably lacking "@MAC-ALLOW:AXSubrole=*" of
> landmark.html.

Yes, that should work.

> As content/ was refactored, I will update the patch set.

Yes, and sorry about the unfortunate timing. Hopefully it's an easy change.

[Takeshi Kurosawa, 2014-01-07 23:06:04]

On 2014/01/07 16:22:31, Dominic Mazzoni wrote:
> > As content/ was refactored, I will update the patch set.
> 
> Yes, and sorry about the unfortunate timing. Hopefully it's an easy change.

I uploaded the rebased patch set.

[dmazzoni, 2014-01-07 23:14:43]

lgtm

[commit-bot: I haz the power, 2014-01-07 23:17:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/taken.spc@gmail.com/98183011/380001

[commit-bot: I haz the power, 2014-01-07 23:42:30]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[dmazzoni, 2014-01-07 23:48:43]

** Presubmit Warnings **
taken.spc@gmail.com is not in AUTHORS file.

You already signed the CLA, you just need to add your
email address to AUTHORS.

[Takeshi Kurosawa, 2014-01-08 04:04:50]

On 2014/01/07 23:48:43, Dominic Mazzoni wrote:
> ** Presubmit Warnings **
> mailto:taken.spc@gmail.com is not in AUTHORS file.
> 
> You already signed the CLA, you just need to add your
> email address to AUTHORS.

I addressed gcl presubmit errors and uploaded latest patch set.

[commit-bot: I haz the power, 2014-01-08 04:45:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/taken.spc@gmail.com/98183011/580001

[commit-bot: I haz the power, 2014-01-08 05:53:09]

Retried try job too often on win_rel for step(s) nacl_integration
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-08 06:29:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/taken.spc@gmail.com/98183011/580001

[commit-bot: I haz the power, 2014-01-08 12:05:12]

Change committed as 243522