Moved action verbs out of Blink.

Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports, instead of returning a string with an action verb, it now returns a value from the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they please. More specifically, on Windows we now expose two APIs: one for returning the localized action verb and one that returns the verb in English. This is a requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org, avi@chromium.org

Committed: https://crrev.com/6e043304856897ee42289b94181baf34726f83c2
Cr-Commit-Position: refs/heads/master@{#438649}

[dmazzoni, 2016-11-22 01:01:09]

https://codereview.chromium.org/2518183002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/2518183002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_win.cc:2890: base::string16
action_verb = base::UTF8ToUTF16(
I think I'd prefer if the names of the enum (which are typically allowed to
be arbitrary within our source code) weren't so closely tied to the
magic strings we're supposed to return on Windows. I think you should
have a little helper function inside browser_accessibility_win that switches
on the action verb and returns the Windows-specific action verb,
with a comment indicating these are part of the contract with screen
readers and that they should not be localized.

Is there a spec somewhere that defines the specific actions we're
supposed to expose, like "click"? I couldn't find one. If so please
reference it.

https://codereview.chromium.org/2518183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/accessibility/AXObject.h (right):

https://codereview.chromium.org/2518183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/accessibility/AXObject.h:250: NONE = 0,
Use CamelCase for these like other enums in this source file.

https://codereview.chromium.org/2518183002/diff/1/third_party/WebKit/public/w...
File third_party/WebKit/public/web/WebAXEnums.h (right):

https://codereview.chromium.org/2518183002/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebAXEnums.h:229: NONE = 0,
These also need to match the style in this source file, so for
example: WebAXSupportedActionNone, WebAXSupportedActionActivate, etc.

https://codereview.chromium.org/2518183002/diff/1/ui/accessibility/ax_enums.idl
File ui/accessibility/ax_enums.idl (right):

https://codereview.chromium.org/2518183002/diff/1/ui/accessibility/ax_enums.i...
ui/accessibility/ax_enums.idl:283: // Lists the actions that can be performed on
a given element.
Please add a comment explaining the difference between AXAction and
AXSupportedAction. AXAction is for the actions we implement that
actually have different semantics internally.

AXSupportedAction is a hint as to what will happen when you trigger
an action, like "jump", "activate", or "uncheck" - those all map to the
same AXAction in terms of how we implement them.

[blink-reviews, 2016-11-23 05:06:49]

All done, but I couldn't find a reference of the action names.
Perhaps I should look through the NVDA or Firefox source code?
I tried going through the Firefox one, but didn't find anything as of yet.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-11-23 05:06:50]

All done, but I couldn't find a reference of the action names.
Perhaps I should look through the NVDA or Firefox source code?
I tried going through the Firefox one, but didn't find anything as of yet.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nektarios, 2016-11-28 16:16:09]

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

[commit-bot: I haz the power, 2016-11-28 16:16:29]

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, 2016-11-28 16:19:50]

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

[commit-bot: I haz the power, 2016-11-28 16:19:51]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-11-28 16:33:30]

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

[commit-bot: I haz the power, 2016-11-28 16:33:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nektarios, 2016-11-28 16:36:29]

Description was changed from

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org
==========

to

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org
==========

[nektarios, 2016-11-28 16:36:29]

nektar@chromium.org changed reviewers:
+ sky@chromium.org

[nektarios, 2016-11-28 16:37:12]

+sky for ui/views and chrome/browser/ui/views

[commit-bot: I haz the power, 2016-11-28 16:37:19]

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

[commit-bot: I haz the power, 2016-11-28 16:37:19]

Dry run: Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/blimp_linux_dbg...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmazzoni, 2016-11-28 17:01:39]

Try bots are all red, do you need to rebase?

[nektarios, 2016-11-28 17:01:41]

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

[commit-bot: I haz the power, 2016-11-28 17:02:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dmazzoni, 2016-11-28 17:03:32]

https://codereview.chromium.org/2518183002/diff/80001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/2518183002/diff/80001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:4067: return
L"Click";
I think NVDA expected "click", lowercase.
Most of these actions came from WebKit, I think
Windows may be simpler and we may want to return
"click" for many of them. Can you just check what
Firefox does for a link, a checked checkbox, an
unchecked checkbox, and a <select> control?

[commit-bot: I haz the power, 2016-11-28 17:15:00]

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

[commit-bot: I haz the power, 2016-11-28 17:15:01]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-11-28 17:59:31]

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

[commit-bot: I haz the power, 2016-11-28 17:59:54]

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, 2016-11-28 18:11:45]

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

[commit-bot: I haz the power, 2016-11-28 18:11:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[sky, 2016-11-28 18:18:01]

https://codereview.chromium.org/2518183002/diff/100001/ui/views/controls/butt...
File ui/views/controls/button/menu_button.cc (right):

https://codereview.chromium.org/2518183002/diff/100001/ui/views/controls/butt...
ui/views/controls/button/menu_button.cc:301:
node_data->AddIntAttribute(ui::AX_ATTR_ACTION, ui::AX_SUPPORTED_ACTION_PRESS);
Does whether the button is enabled make a different as to whether this attribute
is added? Same question for the change to ToolbarButton.

[blink-reviews, 2016-11-28 21:51:56]

On 11/28/2016 1:18 PM, sky@chromium.org wrote:
>
>
https://codereview.chromium.org/2518183002/diff/100001/ui/views/controls/butt...
> File ui/views/controls/button/menu_button.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/100001/ui/views/controls/butt...
> ui/views/controls/button/menu_button.cc:301:
> node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
> ui::AX_SUPPORTED_ACTION_PRESS);
> Does whether the button is enabled make a different as to whether this
> attribute is added? Same question for the change to ToolbarButton.

Indeed, it does.
I modified the code accordingly.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-11-28 21:51:56]

On 11/28/2016 1:18 PM, sky@chromium.org wrote:
>
>
https://codereview.chromium.org/2518183002/diff/100001/ui/views/controls/butt...
> File ui/views/controls/button/menu_button.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/100001/ui/views/controls/butt...
> ui/views/controls/button/menu_button.cc:301:
> node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
> ui::AX_SUPPORTED_ACTION_PRESS);
> Does whether the button is enabled make a different as to whether this
> attribute is added? Same question for the change to ToolbarButton.

Indeed, it does.
I modified the code accordingly.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nektarios, 2016-11-28 21:55:51]

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

[commit-bot: I haz the power, 2016-11-28 21:57:29]

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, 2016-11-28 22:13:10]

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

[commit-bot: I haz the power, 2016-11-28 22:13:11]

Dry run: 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...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sky, 2016-11-28 22:32:00]

https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/butt...
File ui/views/controls/button/checkbox.cc (right):

https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/butt...
ui/views/controls/button/checkbox.cc:128:
node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
Shouldn't the check/uncheck only be added if enabled?

https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/comb...
File ui/views/controls/combobox/combobox.cc (right):

https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/comb...
ui/views/controls/combobox/combobox.cc:743:
node_data->AddIntAttribute(ui::AX_ATTR_ACTION, ui::AX_SUPPORTED_ACTION_OPEN);
Similar comment about enabled.

[nektarios, 2016-11-28 23:02:06]

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

[blink-reviews, 2016-11-28 23:02:51]

On 11/28/2016 5:31 PM, sky@chromium.org wrote:
>
>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/butt...
> File ui/views/controls/button/checkbox.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/butt...
> ui/views/controls/button/checkbox.cc:128:
> node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
> Shouldn't the check/uncheck only be added if enabled?
>
Done.

>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/comb...
> File ui/views/controls/combobox/combobox.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/comb...
> ui/views/controls/combobox/combobox.cc:743:
> node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
> ui::AX_SUPPORTED_ACTION_OPEN);
> Similar comment about enabled.

  Similar done.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-11-28 23:02:52]

On 11/28/2016 5:31 PM, sky@chromium.org wrote:
>
>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/butt...
> File ui/views/controls/button/checkbox.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/butt...
> ui/views/controls/button/checkbox.cc:128:
> node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
> Shouldn't the check/uncheck only be added if enabled?
>
Done.

>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/comb...
> File ui/views/controls/combobox/combobox.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/140001/ui/views/controls/comb...
> ui/views/controls/combobox/combobox.cc:743:
> node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
> ui::AX_SUPPORTED_ACTION_OPEN);
> Similar comment about enabled.

  Similar done.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
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, 2016-11-28 23:04:48]

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, 2016-11-28 23:30:05]

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

[commit-bot: I haz the power, 2016-11-28 23:30:06]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sky, 2016-11-29 00:07:08]

LGTM

[dmazzoni, 2016-11-29 16:28:11]

lgtm

https://codereview.chromium.org/2518183002/diff/160001/content/browser/access...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/2518183002/diff/160001/content/browser/access...
content/browser/accessibility/browser_accessibility_win.cc:4060: case
ui::AX_SUPPORTED_ACTION_NONE:
This isn't a valid value, right? I think this should be NOTREACHED.

https://codereview.chromium.org/2518183002/diff/160001/ui/accessibility/ax_no...
File ui/accessibility/ax_node_data.cc (right):

https://codereview.chromium.org/2518183002/diff/160001/ui/accessibility/ax_no...
ui/accessibility/ax_node_data.cc:396: result += " action=" + value;
Please print this as a string, something like

ui::ToString(static_cast<ui::AXSupportedAction>(value))

[chromium-reviews, 2016-11-29 21:22:15]

On 11/29/2016 11:28 AM, dmazzoni@chromium.org wrote:
> lgtm
>
>
>
>
>
https://codereview.chromium.org/2518183002/diff/160001/content/browser/access...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/160001/content/browser/access...
> content/browser/accessibility/browser_accessibility_win.cc:4060: case
> ui::AX_SUPPORTED_ACTION_NONE:
> This isn't a valid value, right? I think this should be NOTREACHED.
>
Yes, it's not a valid value for now, but I am concerned that if I put 
"NOTREACHED" there and in the future someone copy/pastes this function 
to another class such as a superclass or simply tries to use it for 
converting the values of the enum to an unlocalized string, it will 
crash the browser unexpectedly.
The name of the function is ActionToUnlocalizedString, and action means 
the ui::AXSupportedAction enum. It doesn't come with any pre-conditions.

>
https://codereview.chromium.org/2518183002/diff/160001/ui/accessibility/ax_no...
> File ui/accessibility/ax_node_data.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/160001/ui/accessibility/ax_no...
> ui/accessibility/ax_node_data.cc:396: result += " action=" + value;
> Please print this as a string, something like
>
> ui::ToString(static_cast<ui::AXSupportedAction>(value))
Done.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[blink-reviews, 2016-11-29 21:22:15]

On 11/29/2016 11:28 AM, dmazzoni@chromium.org wrote:
> lgtm
>
>
>
>
>
https://codereview.chromium.org/2518183002/diff/160001/content/browser/access...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/160001/content/browser/access...
> content/browser/accessibility/browser_accessibility_win.cc:4060: case
> ui::AX_SUPPORTED_ACTION_NONE:
> This isn't a valid value, right? I think this should be NOTREACHED.
>
Yes, it's not a valid value for now, but I am concerned that if I put 
"NOTREACHED" there and in the future someone copy/pastes this function 
to another class such as a superclass or simply tries to use it for 
converting the values of the enum to an unlocalized string, it will 
crash the browser unexpectedly.
The name of the function is ActionToUnlocalizedString, and action means 
the ui::AXSupportedAction enum. It doesn't come with any pre-conditions.

>
https://codereview.chromium.org/2518183002/diff/160001/ui/accessibility/ax_no...
> File ui/accessibility/ax_node_data.cc (right):
>
>
https://codereview.chromium.org/2518183002/diff/160001/ui/accessibility/ax_no...
> ui/accessibility/ax_node_data.cc:396: result += " action=" + value;
> Please print this as a string, something like
>
> ui::ToString(static_cast<ui::AXSupportedAction>(value))
Done.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[nektarios, 2016-11-29 21:26:10]

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

[commit-bot: I haz the power, 2016-11-29 21:26:56]

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, 2016-11-29 21:50:08]

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

[commit-bot: I haz the power, 2016-11-29 21:50:10]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[nektarios, 2016-11-29 22:31:14]

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

[commit-bot: I haz the power, 2016-11-29 22:32:37]

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, 2016-11-29 23:49:41]

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

[commit-bot: I haz the power, 2016-11-29 23:49:42]

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

[nektarios, 2016-11-30 01:51:29]

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

[commit-bot: I haz the power, 2016-11-30 01:52:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nektarios, 2016-11-30 03:01:14]

The CQ bit was unchecked by nektar@chromium.org

[nektarios, 2016-11-30 03:01:33]

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

[commit-bot: I haz the power, 2016-11-30 03:02:31]

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, 2016-11-30 03:41:28]

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

[commit-bot: I haz the power, 2016-11-30 03:41:29]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[nektarios, 2016-12-08 19:23:47]

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

[commit-bot: I haz the power, 2016-12-08 19:24:42]

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, 2016-12-08 19:52:19]

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

[commit-bot: I haz the power, 2016-12-08 19:52:20]

Dry run: 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_...)

[nektarios, 2016-12-08 20:35:23]

It turns out that Linux ATK also had the same requirements, so I moved
ActionToUnlocalizedName to the base class - BrowserAccessibility.
I didn't implement get_localizedActionName for Linux ATK. We should do that in
the future.

[nektarios, 2016-12-08 20:35:43]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-12-08 20:35:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2518183002/#ps240001
(title: "Fixed get_actionName on Linux.")

[commit-bot: I haz the power, 2016-12-08 20:37:03]

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, 2016-12-08 21:10:16]

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

[commit-bot: I haz the power, 2016-12-08 21:10:18]

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

[nektarios, 2016-12-08 22:12:09]

Description was changed from

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org
==========

to

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org, avi@chromium.org
==========

[nektarios, 2016-12-08 22:12:09]

nektar@chromium.org changed reviewers:
+ avi@chromium.org

[nektarios, 2016-12-08 22:13:30]

I need approval from owners to content/app and content/child.

[Avi (use Gerrit), 2016-12-09 15:54:15]

lgtm

stamp

[nektarios, 2016-12-09 20:44:31]

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

[commit-bot: I haz the power, 2016-12-09 20:45:17]

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, 2016-12-09 21:49:50]

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

[commit-bot: I haz the power, 2016-12-09 21:49:52]

Dry run: 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_...)

[nektarios, 2016-12-09 23:02:11]

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

[commit-bot: I haz the power, 2016-12-09 23:03:46]

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, 2016-12-10 00:10:19]

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

[commit-bot: I haz the power, 2016-12-10 00:10:20]

Dry run: 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_...)

[nektarios, 2016-12-10 02:21:29]

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

[commit-bot: I haz the power, 2016-12-10 02:22:10]

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, 2016-12-10 03:16:15]

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

[commit-bot: I haz the power, 2016-12-10 03:16:17]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[nektarios, 2016-12-12 16:48:22]

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

[commit-bot: I haz the power, 2016-12-12 16:48:39]

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, 2016-12-12 17:53:41]

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

[commit-bot: I haz the power, 2016-12-12 17:53:42]

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

[nektarios, 2016-12-12 19:56:00]

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

[commit-bot: I haz the power, 2016-12-12 19:57:10]

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, 2016-12-12 20:11:03]

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

[commit-bot: I haz the power, 2016-12-12 20:11:04]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[nektarios, 2016-12-12 22:26:03]

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

[commit-bot: I haz the power, 2016-12-12 22:27:12]

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, 2016-12-12 23:56:52]

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

[commit-bot: I haz the power, 2016-12-12 23:56:54]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-12-13 03:33:15]

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

[commit-bot: I haz the power, 2016-12-13 03:33:53]

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, 2016-12-13 04:24:03]

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

[commit-bot: I haz the power, 2016-12-13 04:24:05]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[nektarios, 2016-12-13 14:47:19]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-12-13 14:47:20]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
dmazzoni@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2518183002/#ps380001
(title: "Fixed test expectations.")

[commit-bot: I haz the power, 2016-12-13 14:47:32]

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, 2016-12-13 18:05:33]

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

[commit-bot: I haz the power, 2016-12-13 18:05:35]

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_...)

[nektarios, 2016-12-13 21:55:49]

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

[commit-bot: I haz the power, 2016-12-13 21:56:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dmazzoni, 2016-12-13 23:24:34]

lgtm

https://codereview.chromium.org/2518183002/diff/400001/ui/accessibility/ax_en...
File ui/accessibility/ax_enums.idl (right):

https://codereview.chromium.org/2518183002/diff/400001/ui/accessibility/ax_en...
ui/accessibility/ax_enums.idl:240: // In contrast to |AXSupportedAction|, these
describe what happens to the object, e.g. "FOCUS".
nit: wrap

https://codereview.chromium.org/2518183002/diff/400001/ui/accessibility/ax_en...
ui/accessibility/ax_enums.idl:297: // In contrast to |AXAction|, these describe
what the user can do on the object, e.g. "PRESS", not what happens to the object
as a result.
nit: wrap

https://codereview.chromium.org/2518183002/diff/400001/ui/accessibility/ax_te...
File ui/accessibility/ax_text_utils.h (right):

https://codereview.chromium.org/2518183002/diff/400001/ui/accessibility/ax_te...
ui/accessibility/ax_text_utils.h:60: // Some APIs on Linux and Windows need to
return non-localized action names.
What are we going to do if the name of the string
we want to expose ends up being different on Linux
and Windows?

This is fine for now, but I think it may have to
be refactored if we end up with that problem.

[commit-bot: I haz the power, 2016-12-13 23:46:26]

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

[commit-bot: I haz the power, 2016-12-13 23:46:27]

Dry run: This issue passed the CQ dry run.

[nektarios, 2016-12-14 00:19:25]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-12-14 00:19:26]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
sky@chromium.org, dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2518183002/#ps420001
(title: "Fixed long lines.")

[nektarios, 2016-12-14 00:19:29]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-12-14 00:20:06]

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, 2016-12-14 03:04:54]

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

[commit-bot: I haz the power, 2016-12-14 03:04:59]

Failed to apply patch for chrome/browser/ui/views/toolbar/toolbar_button.cc:
While running git apply --index -p1;
  error: patch failed: chrome/browser/ui/views/toolbar/toolbar_button.cc:15
  error: chrome/browser/ui/views/toolbar/toolbar_button.cc: patch does not apply

Patch:       chrome/browser/ui/views/toolbar/toolbar_button.cc
Index: chrome/browser/ui/views/toolbar/toolbar_button.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_button.cc
b/chrome/browser/ui/views/toolbar/toolbar_button.cc
index
00e427c500ba6dfb5d3778db8621be4d9013d3e2..08232fd28ff7cd72f5f281c3768f86f7af9069c4
100644
--- a/chrome/browser/ui/views/toolbar/toolbar_button.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_button.cc
@@ -15,11 +15,9 @@
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "ui/accessibility/ax_node_data.h"
-#include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
-#include "ui/strings/grit/ui_strings.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_model_adapter.h"
@@ -139,9 +137,10 @@ void ToolbarButton::OnGestureEvent(ui::GestureEvent* event)
{
 void ToolbarButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   CustomButton::GetAccessibleNodeData(node_data);
   node_data->role = ui::AX_ROLE_BUTTON_DROP_DOWN;
-  node_data->AddStringAttribute(
-      ui::AX_ATTR_ACTION, l10n_util::GetStringUTF8(IDS_APP_ACCACTION_PRESS));
   node_data->AddStateFlag(ui::AX_STATE_HASPOPUP);
+  if (enabled())
+    node_data->AddIntAttribute(ui::AX_ATTR_ACTION,
+                               ui::AX_SUPPORTED_ACTION_PRESS);
 }
 
 std::unique_ptr<views::LabelButtonBorder> ToolbarButton::CreateDefaultBorder()

[nektarios, 2016-12-14 18:24:39]

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

[commit-bot: I haz the power, 2016-12-14 18:25:19]

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, 2016-12-14 20:00:28]

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

[commit-bot: I haz the power, 2016-12-14 20:00:31]

Dry run: 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_...)

[nektarios, 2016-12-14 20:08:55]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-12-14 20:08:56]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
dmazzoni@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2518183002/#ps440001
(title: "Added missing braces.")

[commit-bot: I haz the power, 2016-12-14 20:09:33]

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, 2016-12-14 22:22:03]

CQ is committing da patch.

Bot data: {"patchset_id": 440001, "attempt_start_ts": 1481746135691830,
"parent_rev": "f1275ee45b7c6aa167900c2fac5f7515f6f1df91", "commit_rev":
"6f211ac5b7447dd967884e1a8cfdc9b5658cfd22"}

[commit-bot: I haz the power, 2016-12-14 22:22:31]

Description was changed from

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org, avi@chromium.org
==========

to

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org, avi@chromium.org

Review-Url: https://codereview.chromium.org/2518183002
==========

[commit-bot: I haz the power, 2016-12-14 22:22:35]

Committed patchset #23 (id:440001)

[commit-bot: I haz the power, 2016-12-14 22:25:02]

Description was changed from

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org, avi@chromium.org

Review-Url: https://codereview.chromium.org/2518183002
==========

to

==========
Moved action verbs out of Blink.

When Blink is asked to provide what actions an accessibility object supports,
instead of returning a string with an action verb, it now returns a value from
the |WebAXSupportedAction| enumeration.
This change allows different platforms to localize the actions in any way they
please. More specifically, on Windows we now expose two APIs: one for returning
the localized action verb and one that returns the verb in English. This is a
requirement dictated by the IAccessible2 Spec.
BUG=640043
TESTED=By changing Chrome's language to something other than English and trying
to read a clickable element with NVDA
R=dmazzoni@chromium.org, sky@chromium.org, avi@chromium.org

Committed: https://crrev.com/6e043304856897ee42289b94181baf34726f83c2
Cr-Commit-Position: refs/heads/master@{#438649}
==========

[commit-bot: I haz the power, 2016-12-14 22:25:04]

Patchset 23 (id:??) landed as
https://crrev.com/6e043304856897ee42289b94181baf34726f83c2
Cr-Commit-Position: refs/heads/master@{#438649}