Forward BrowserAccessibility get_accRole to AXPlatformNode.

Forward BrowserAccessibility get_accRole to AXPlatformNode.

This is another migration from BrowserAccessibility to AXPlatformNodeWin.
In this installment, we're converting get_accRole.  I have updated the
AXPlatformNodeWin::MSAARole() to include many of the missing roles that
the BrowserAccessibility code had.  I also introduced a new method that
allows get_accRole to return a string (overriding the default type value
that MSAARole() returns).  I have not dug into why we need to return the
various values instead of just using the role number or html element
name. Instead, I just added todos for me to follow up on.

BUG=703369
Review-Url: https://codereview.chromium.org/2909853002
Cr-Commit-Position: refs/heads/master@{#475757}
Committed: https://chromium.googlesource.com/chromium/src/+/ebe52af6c0285feef6f776dfd95669123315e525

[dougt, 2017-05-28 21:07:51]

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

[commit-bot: I haz the power, 2017-05-28 21:08:03]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dougt, 2017-05-28 22:34:36]

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

[commit-bot: I haz the power, 2017-05-28 22:34:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-28 23:28:34]

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

[commit-bot: I haz the power, 2017-05-28 23:28:34]

Dry run: This issue passed the CQ dry run.

[dougt, 2017-05-29 02:31:13]

dougt@chromium.org changed reviewers:
+ dmazzoni@google.com

[dougt, 2017-05-29 02:31:13]

Hi dmazzoni, please take a look.

[dougt, 2017-05-29 05:09:34]

Description was changed from

==========
Forward BrowserAccessibility get_accRole to AXPlatformNode.

This is another migration from BrowserAccessibility to AXPlatformNodeWin.
In this installment, we're converting get_accRole.  I have updated the
AXPlatformNodeWin::MSAARole() to include many of the missing roles that
the BrowserAccessibility code had.  I also introduced a new method that
allows get_accRole to return a string (overriding the default type value
that MSAARole() returns).  I have not dug into why we need to return the
various values instead of just using the role number or html element
name. Instead, I just added todos for me to follow up on.

BUG=703369
==========

to

==========
Forward BrowserAccessibility get_accRole to AXPlatformNode.

This is another migration from BrowserAccessibility to AXPlatformNodeWin.
In this installment, we're converting get_accRole.  I have updated the
AXPlatformNodeWin::MSAARole() to include many of the missing roles that
the BrowserAccessibility code had.  I also introduced a new method that
allows get_accRole to return a string (overriding the default type value
that MSAARole() returns).  I have not dug into why we need to return the
various values instead of just using the role number or html element
name. Instead, I just added todos for me to follow up on.

BUG=703369
==========

[dougt, 2017-05-29 05:09:35]

dougt@chromium.org changed reviewers:
+ dmazzoni@chromium.org
- dmazzoni@google.com

[dmazzoni, 2017-05-30 15:17:34]

lgtm

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
File ui/accessibility/platform/ax_platform_node_win.cc (right):

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
ui/accessibility/platform/ax_platform_node_win.cc:503: // Some MSAA roles
require us to return a specific string or the html
I'd say this instead: For historical reasons, we return a string (typically
containing the HTML tag name) as the MSAA role, rather than a int.

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
ui/accessibility/platform/ax_platform_node_win.cc:506: if (role_string != "") {
if (!role_string.empty())

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
ui/accessibility/platform/ax_platform_node_win.cc:1382: // TODO(dougt) why not
just use the html_tag?
I'm fine with html_tag

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
ui/accessibility/platform/ax_platform_node_win.cc:1388: // TODO(dougt) why not
just use the html_tag?
This could be <div role="form">

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
ui/accessibility/platform/ax_platform_node_win.cc:1397: // TODO(dougt) why not
just use the html_tag and why upper case?
I'm fine with html_tag

[dougt, 2017-05-31 02:00:18]

Patchset #4 (id:60001) has been deleted

[dougt, 2017-05-31 02:03:00]

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

[commit-bot: I haz the power, 2017-05-31 02:03:44]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dougt, 2017-05-31 02:05:14]

I'd like to fix the todo's in a follow up.  My reasoning is that I suspect I
will end up having to change some tests files since the case of the strings that
 get_accRole() can return is not consistent.  For example,  paragraphs as you
saw we use "P".  But for other things like header elements, our tests tests
against lower case.  WTDY?

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
File ui/accessibility/platform/ax_platform_node_win.cc (right):

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
ui/accessibility/platform/ax_platform_node_win.cc:503: // Some MSAA roles
require us to return a specific string or the html
On 2017/05/30 15:17:34, dmazzoni wrote:
> I'd say this instead: For historical reasons, we return a string (typically
> containing the HTML tag name) as the MSAA role, rather than a int.

Done.

https://codereview.chromium.org/2909853002/diff/20001/ui/accessibility/platfo...
ui/accessibility/platform/ax_platform_node_win.cc:506: if (role_string != "") {
On 2017/05/30 15:17:34, dmazzoni wrote:
> if (!role_string.empty())

Done.

[dougt, 2017-05-31 02:45:31]

On 2017/05/31 02:05:14, dougt wrote:
> I'd like to fix the todo's in a follow up.  My reasoning is that I suspect I
> will end up having to change some tests files since the case of the strings
that
>  get_accRole() can return is not consistent.  For example,  paragraphs as you
> saw we use "P".  But for other things like header elements, our tests tests
> against lower case.  WTDY?


followup: https://codereview.chromium.org/2912243002/

[dougt, 2017-05-31 02:45:52]

The CQ bit was unchecked by dougt@chromium.org

[dougt, 2017-05-31 02:45:58]

The CQ bit was checked by dougt@chromium.org

[dougt, 2017-05-31 02:45:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2909853002/#ps80001
(title: "some of dmazzoni's comments addressed")

[commit-bot: I haz the power, 2017-05-31 02:46:18]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 02:50:53]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1496198758208870,
"parent_rev": "1e68f60e90fc7b0b12bbf884291458e5e4aa3b2f", "commit_rev":
"ebe52af6c0285feef6f776dfd95669123315e525"}

[commit-bot: I haz the power, 2017-05-31 02:51:04]

Description was changed from

==========
Forward BrowserAccessibility get_accRole to AXPlatformNode.

This is another migration from BrowserAccessibility to AXPlatformNodeWin.
In this installment, we're converting get_accRole.  I have updated the
AXPlatformNodeWin::MSAARole() to include many of the missing roles that
the BrowserAccessibility code had.  I also introduced a new method that
allows get_accRole to return a string (overriding the default type value
that MSAARole() returns).  I have not dug into why we need to return the
various values instead of just using the role number or html element
name. Instead, I just added todos for me to follow up on.

BUG=703369
==========

to

==========
Forward BrowserAccessibility get_accRole to AXPlatformNode.

This is another migration from BrowserAccessibility to AXPlatformNodeWin.
In this installment, we're converting get_accRole.  I have updated the
AXPlatformNodeWin::MSAARole() to include many of the missing roles that
the BrowserAccessibility code had.  I also introduced a new method that
allows get_accRole to return a string (overriding the default type value
that MSAARole() returns).  I have not dug into why we need to return the
various values instead of just using the role number or html element
name. Instead, I just added todos for me to follow up on.

BUG=703369

Review-Url: https://codereview.chromium.org/2909853002
Cr-Commit-Position: refs/heads/master@{#475757}
Committed:
https://chromium.googlesource.com/chromium/src/+/ebe52af6c0285feef6f776dfd956...
==========

[commit-bot: I haz the power, 2017-05-31 02:51:06]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/ebe52af6c0285feef6f776dfd956...