Update AX role mapping for summary to AXButton on Mac

Update AX role mapping for summary to AXButton on Mac

The summary element should have AXDisclosureTriangle as the spec described
at https://www.w3.org/TR/html-aam-1.0/#el-summary but Mac doesn't handle
AXExpanded event with AXDisclosureTriangle.
As WebKit on Mac assigns AXButton to summary and updates expanded state
correctly with Voice Over, Chromium will follows role mapping for summary
from WebKit until Mac works with AXExpanded event on AXDisclosureTriangle.

BUG=558324
Review-Url: https://codereview.chromium.org/2948883003
Cr-Commit-Position: refs/heads/master@{#482884}
Committed: https://chromium.googlesource.com/chromium/src/+/8cf9ccb3cb8f650ab385ec2d359fcf27e76abf77

[je_julie, 2017-06-21 13:48:57]

je_julie.kim@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[je_julie, 2017-06-21 13:48:57]

Hi Dominic,

This patch changed summary role for Mac to get AXExpand Event on Mac like
WebKit. I confirmed VoiceOver on Mac works correctly with AXButton Role for
summary on updating expand state with pressing keyboard space bar.

Here is test step.
1) Load web page which has summary element on Mac.
2) Turn on Voice Over.
3) Change expand state of summary role with pressing space bar.
=> VoiceOver updates expand state.
Previously it doesn't speak anything even if summary is expanded or collapsed.

PTAL.

[dmazzoni, 2017-06-23 18:20:11]

lgtm and thanks, with one suggestion to make it even
better

https://codereview.chromium.org/2948883003/diff/1/content/test/data/accessibi...
File content/test/data/accessibility/html/details-expected-mac.txt (right):

https://codereview.chromium.org/2948883003/diff/1/content/test/data/accessibi...
content/test/data/accessibility/html/details-expected-mac.txt:3: ++++AXButton
AXRoleDescription='button' AXTitle='details tag' AXExpanded='0'
Would it work if we used a role of "button" but used the role description of
"disclosure triangle"? That might be even better.

Specifically I mean that the role description would be our own
localized string in Chrome. That way VoiceOver users would hear
"disclosure triangle" but the feedback would match a button
otherwise.

https://codereview.chromium.org/2948883003/diff/1/ui/accessibility/platform/a...
File ui/accessibility/platform/ax_platform_node_mac.mm (right):

https://codereview.chromium.org/2948883003/diff/1/ui/accessibility/platform/a...
ui/accessibility/platform/ax_platform_node_mac.mm:512: case ui::AX_ROLE_TAB:
We could add the custom role description here, and to the
equivalent method in browser_accessibility_cocoa.mm

[je_julie, 2017-06-26 12:27:36]

je_julie.kim@chromium.org changed reviewers:
+ jcivelli@chromium.org

[je_julie, 2017-06-26 12:27:37]

Hi Dominic,
Thanks for the review. Please take a look again.

For content/app/strings/content_strings.grd,
jcivelli, PTAL.

https://codereview.chromium.org/2948883003/diff/1/content/test/data/accessibi...
File content/test/data/accessibility/html/details-expected-mac.txt (right):

https://codereview.chromium.org/2948883003/diff/1/content/test/data/accessibi...
content/test/data/accessibility/html/details-expected-mac.txt:3: ++++AXButton
AXRoleDescription='button' AXTitle='details tag' AXExpanded='0'
On 2017/06/23 18:20:11, dmazzoni wrote:
> Would it work if we used a role of "button" but used the role description of
> "disclosure triangle"? That might be even better.
> 
> Specifically I mean that the role description would be our own
> localized string in Chrome. That way VoiceOver users would hear
> "disclosure triangle" but the feedback would match a button
> otherwise.

You're right. Thanks for the great idea.

https://codereview.chromium.org/2948883003/diff/1/ui/accessibility/platform/a...
File ui/accessibility/platform/ax_platform_node_mac.mm (right):

https://codereview.chromium.org/2948883003/diff/1/ui/accessibility/platform/a...
ui/accessibility/platform/ax_platform_node_mac.mm:512: case ui::AX_ROLE_TAB:
On 2017/06/23 18:20:11, dmazzoni wrote:
> We could add the custom role description here, and to the
> equivalent method in browser_accessibility_cocoa.mm

I updated role description here and browser_accessibility_cocoa.mm.
Can I ask something? I can see role description works enough well with
browser_accessibility_cocoa.mm change. I couldn't find where this function's
used. Could you let me know?

[dmazzoni, 2017-06-26 16:46:45]

lgtm

[Jay Civelli, 2017-06-26 16:52:10]

lgtm

[je_julie, 2017-06-27 00:48:48]

The CQ bit was checked by je_julie.kim@chromium.org

[commit-bot: I haz the power, 2017-06-27 00:49:07]

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-06-27 01:11:07]

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

[commit-bot: I haz the power, 2017-06-27 01:11:09]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[je_julie, 2017-06-27 08:19:37]

On 2017/06/27 01:11:09, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
>
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

Hi Jay,
I updated content/app/strings/content_strings.grd.
As I realized that IDS_AX_ROLE_DISCLOSURE_TRIANGLE was defined already for
Android, I made it common with reference to IDS_AX_ROLE_DESCRIPTION_TERM.
Could you take a look again?
Failing on android_compile_rel doesn’t seem to be related to this change.
Thanks,

[Jay Civelli, 2017-06-27 15:36:45]

Still LGTM

[je_julie, 2017-06-28 00:22:20]

The CQ bit was checked by je_julie.kim@chromium.org

[je_julie, 2017-06-28 00:22:21]

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/2948883003/#ps40001
(title: "Fixed duplicate resources error.")

[commit-bot: I haz the power, 2017-06-28 00:22:59]

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-06-28 02:32:19]

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

[commit-bot: I haz the power, 2017-06-28 02:32:21]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[je_julie, 2017-06-28 04:47:33]

The CQ bit was checked by je_julie.kim@chromium.org

[commit-bot: I haz the power, 2017-06-28 04:47:47]

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-06-28 04:51:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498625253061330,
"parent_rev": "159b6ee709a6fb2ef416f7c10e39c8c706e92206", "commit_rev":
"8cf9ccb3cb8f650ab385ec2d359fcf27e76abf77"}

[commit-bot: I haz the power, 2017-06-28 04:51:31]

Description was changed from

==========
Update AX role mapping for summary to AXButton on Mac

The summary element should have AXDisclosureTriangle as the spec described
at https://www.w3.org/TR/html-aam-1.0/#el-summary but Mac doesn't handle
AXExpanded event with AXDisclosureTriangle.
As WebKit on Mac assigns AXButton to summary and updates expanded state
correctly with Voice Over, Chromium will follows role mapping for summary
from WebKit until Mac works with AXExpanded event on AXDisclosureTriangle.

BUG=558324
==========

to

==========
Update AX role mapping for summary to AXButton on Mac

The summary element should have AXDisclosureTriangle as the spec described
at https://www.w3.org/TR/html-aam-1.0/#el-summary but Mac doesn't handle
AXExpanded event with AXDisclosureTriangle.
As WebKit on Mac assigns AXButton to summary and updates expanded state
correctly with Voice Over, Chromium will follows role mapping for summary
from WebKit until Mac works with AXExpanded event on AXDisclosureTriangle.

BUG=558324

Review-Url: https://codereview.chromium.org/2948883003
Cr-Commit-Position: refs/heads/master@{#482884}
Committed:
https://chromium.googlesource.com/chromium/src/+/8cf9ccb3cb8f650ab385ec2d359f...
==========

[commit-bot: I haz the power, 2017-06-28 04:51:32]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8cf9ccb3cb8f650ab385ec2d359f...