DevTools: Introduce ARIAUtils

DevTools: Introduce ARIAUtils

BUG=none

Review-Url: https://codereview.chromium.org/2655393003
Cr-Commit-Position: refs/heads/master@{#447901}
Committed: https://chromium.googlesource.com/chromium/src/+/652129ce6e65ee501a16614a9ec44b830a56ca0c

[einbinder, 2017-01-26 22:36:01]

einbinder@chromium.org changed reviewers:
+ aboxhall@chromium.org, hans.hillen@gmail.com

[einbinder, 2017-01-26 22:36:01]

This is my slimmed down version of the ARIAUtils patch.

[einbinder, 2017-01-26 22:40:18]

einbinder@chromium.org changed reviewers:
+ dgozman@chromium.org

[aboxhall, 2017-01-26 22:47:26]

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js (right):

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:26:
element.setAttribute('aria-pressed', !!value);
This may not work for all cases. Note that pressed may take 'true', 'false',
'mixed' or 'undefined' as values:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

Also, aria-pressed is not valid for all roles:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

ARIAConfig captures all of this type of logic.

[dgozman, 2017-01-26 23:21:42]

This definitely needs a bug and description.

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js (right):

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:1: // Copyright (c)
2017 The Chromium Authors. All rights reserved.
no (c)

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:5: UI.ARIAUtils =
class {
Are we going to have any logic here? I'm usually not a fan of one-line methods
which don't do anything - we can as well inline them.

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:18:
element.setAttribute('aria-selected', !!value);
Remove !!

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:26:
element.setAttribute('aria-pressed', !!value);
Remove !!

[einbinder, 2017-01-27 00:19:27]

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js (right):

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:1: // Copyright (c)
2017 The Chromium Authors. All rights reserved.
On 2017/01/26 at 23:21:42, dgozman wrote:
> no (c)

Done.

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:5: UI.ARIAUtils =
class {
On 2017/01/26 at 23:21:42, dgozman wrote:
> Are we going to have any logic here? I'm usually not a fan of one-line methods
which don't do anything - we can as well inline them.

It is mostly for type checking, and to have an organized view of all the aria we
use.

https://codereview.chromium.org/2655393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:18:
element.setAttribute('aria-selected', !!value);
On 2017/01/26 at 23:21:42, dgozman wrote:
> Remove !!

We have a lot of bugs with closure where it thinks that undefined values are
booleans. undefined and null have different meanings from false with
aria-selected, so I added !! just to be safe.

[dgozman, 2017-01-27 23:12:59]

lgtm

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js (right):

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:19:
element.setAttribute('aria-selected', !!value);
Let's add a comment about !!

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:27:
element.setAttribute('aria-pressed', !!value);
Let's add a comment about !!

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js (right):

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js:46:
UI.ARIAUtils.setAccessibleName(this._tabsElement, Common.UIString('Panels'));
This should be in InspectorView.

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js:755:
UI.ARIAUtils.setSelected(tab.tabElement, false);
true

[einbinder, 2017-02-02 22:53:32]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2017-02-02 22:53:32]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2655393003/#ps60001
(title: "setAccessibleName in InspectorView")

[einbinder, 2017-02-02 22:53:33]

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js (right):

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:19:
element.setAttribute('aria-selected', !!value);
On 2017/01/27 at 23:12:59, dgozman_slow wrote:
> Let's add a comment about !!

Done.

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ARIAUtils.js:27:
element.setAttribute('aria-pressed', !!value);
On 2017/01/27 at 23:12:59, dgozman_slow wrote:
> Let's add a comment about !!

Done.

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js (right):

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js:46:
UI.ARIAUtils.setAccessibleName(this._tabsElement, Common.UIString('Panels'));
On 2017/01/27 at 23:12:59, dgozman_slow wrote:
> This should be in InspectorView.

Done.

https://codereview.chromium.org/2655393003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js:755:
UI.ARIAUtils.setSelected(tab.tabElement, false);
On 2017/01/27 at 23:12:59, dgozman_slow wrote:
> true

Done.

[einbinder, 2017-02-02 22:53:40]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2017-02-02 22:53:41]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2655393003/#ps60001
(title: "setAccessibleName in InspectorView")

[commit-bot: I haz the power, 2017-02-02 22:54:40]

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-02-03 03:27:56]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486076020774850,
"parent_rev": "90149a4889e454cdca844de420d9deac61b99db6", "commit_rev":
"652129ce6e65ee501a16614a9ec44b830a56ca0c"}

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

Description was changed from

==========
DevTools: Introduce ARIAUtils

BUG=none
==========

to

==========
DevTools: Introduce ARIAUtils

BUG=none

Review-Url: https://codereview.chromium.org/2655393003
Cr-Commit-Position: refs/heads/master@{#447901}
Committed:
https://chromium.googlesource.com/chromium/src/+/652129ce6e65ee501a16614a9ec4...
==========

[commit-bot: I haz the power, 2017-02-03 03:28:34]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/652129ce6e65ee501a16614a9ec4...