Expose aria role menuitemcheckbox and menuitemradio with correct IA2 role.(Chromium)

Expose aria role menuitemcheckbox and menuitemradio with correct IA2 role.(Chromium)

According to specs(http://www.w3.org/WAI/PF/aria-implementation/)

aria role menuitemcheckbox should have IA2 role as IA2_ROLE_CHECK_MENU_ITEM
aria role menuitemradio should have IA2 role as IA2_ROLE_RADIO_MENU_ITEM

This CL depends on https://codereview.chromium.org/629003003/ and https://codereview.chromium.org/631113002/
BUG=169569, 420605, 169573
TBR=kalman

Committed: https://crrev.com/93f1e4e35afa32aefe0ad065e7fe166ca25a9f22
Cr-Commit-Position: refs/heads/master@{#298780}

[shreeramk, 2014-10-08 05:14:48]

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

[shreeramk, 2014-10-08 05:14:49]

Please review.

Thanks!

[dmazzoni, 2014-10-08 06:58:32]

https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
File content/test/data/accessibility/aria-menuitemcheckbox-expected-android.txt
(right):

https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
content/test/data/accessibility/aria-menuitemcheckbox-expected-android.txt:3:
android.view.View clickable name='Menu item 1'
This should be checkable, right?

https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
content/test/data/accessibility/aria-menuitemcheckbox-expected-android.txt:5:
android.view.View clickable name='Menu item 3'
This should be checkable, right?

https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
File content/test/data/accessibility/aria-menuitemcheckbox-expected-mac.txt
(right):

https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
content/test/data/accessibility/aria-menuitemcheckbox-expected-mac.txt:3:
AXMenuItem AXRoleDescription='menu item' AXTitle='Menu item 1'
According to the spec, we're supposed to implement AXMenuItemMarkChar.

Please either do that in this change, or mark this file as skipped for now and
file a new bug for that.

[Mike West, 2014-10-08 08:13:22]

I don't have comments beyond what Dominic's already said. The code change looks
good to me, but I'll defer to him for the review.

[shreeramk, 2014-10-08 08:15:15]

On 2014/10/08 06:58:32, dmazzoni wrote:
>
https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
> File
content/test/data/accessibility/aria-menuitemcheckbox-expected-android.txt
> (right):
> 
>
https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
> content/test/data/accessibility/aria-menuitemcheckbox-expected-android.txt:3:
> android.view.View clickable name='Menu item 1'
> This should be checkable, right?

Once androd_dbg_tests_recipe bot will give the results for android platform.
Then I ll update the expectations for -expected-android.txt

>
https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
> content/test/data/accessibility/aria-menuitemcheckbox-expected-android.txt:5:
> android.view.View clickable name='Menu item 3'
> This should be checkable, right?
> 
>
https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
> File content/test/data/accessibility/aria-menuitemcheckbox-expected-mac.txt
> (right):
> 
>
https://codereview.chromium.org/633043002/diff/20001/content/test/data/access...
> content/test/data/accessibility/aria-menuitemcheckbox-expected-mac.txt:3:
> AXMenuItem AXRoleDescription='menu item' AXTitle='Menu item 1'
> According to the spec, we're supposed to implement AXMenuItemMarkChar.
> 
> Please either do that in this change, or mark this file as skipped for now and
> file a new bug for that.

Filed a new bug and skipped for now.

[shreeramk, 2014-10-08 08:41:48]

PTAL!

Updated expectations for android platform.

Skipped expectations for mac platform and filed a new bug to implement
AXMenuItemMarkChar.

Thanks.

[dmazzoni, 2014-10-08 18:54:06]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2014-10-08 18:54:06]

lgtm

Great, thanks!

[commit-bot: I haz the power, 2014-10-08 18:54:44]

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

[commit-bot: I haz the power, 2014-10-08 19:07:15]

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

[commit-bot: I haz the power, 2014-10-08 19:07:17]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[shreeramk, 2014-10-08 19:49:27]

On 2014/10/08 19:07:17, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)


Chromium_presubmit is failing :(

What could be the cause for this?

[dmazzoni, 2014-10-08 20:01:05]

dmazzoni@chromium.org changed reviewers:
+ kalman@chromium.org

[dmazzoni, 2014-10-08 20:01:06]

+kalman for automation.idl

[dmazzoni, 2014-10-08 20:01:37]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2014-10-08 20:02:36]

>
> Chromium_presubmit is failing :(
>
> What could be the cause for this?


You can see in the logs, it needed an lgtm from an owner of automation.idl
(see the OWNERS files). I have a separate bug to update the ownership for
this file so trivial changes to it like this don't require extra review.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-10-08 20:03:39]

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

[dmazzoni, 2014-10-08 20:04:13]

lgtm

[shreeramk, 2014-10-08 20:09:01]

On 2014/10/08 20:02:36, dmazzoni wrote:
> >
> > Chromium_presubmit is failing :(
> >
> > What could be the cause for this?
> 
> 
> You can see in the logs, it needed an lgtm from an owner of automation.idl
> (see the OWNERS files). I have a separate bug to update the ownership for
> this file so trivial changes to it like this don't require extra review.
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Yea I saw the error now in logs.

[commit-bot: I haz the power, 2014-10-08 20:58:48]

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

[commit-bot: I haz the power, 2014-10-08 20:58:53]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[not at google - send to devlin, 2014-10-09 00:22:51]

lgtm

[shreeramk, 2014-10-09 03:03:14]

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

[commit-bot: I haz the power, 2014-10-09 03:05:21]

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

[commit-bot: I haz the power, 2014-10-09 04:45:27]

Committed patchset #5 (id:80001) as df663e9d92edd1650455c2cf9f6fd7f1582b03e3

[commit-bot: I haz the power, 2014-10-09 04:46:27]

Patchset 5 (id:??) landed as
https://crrev.com/93f1e4e35afa32aefe0ad065e7fe166ca25a9f22
Cr-Commit-Position: refs/heads/master@{#298780}