Add private extension API to move the accessibility focus ring.

Add private extension API to move the accessibility focus ring.

BUG=314889
Committed: https://crrev.com/fecaedc03904c091897dea2926ec00756f6e53ed
Cr-Commit-Position: refs/heads/master@{#294719}

[dmazzoni, 2014-09-10 21:32:52]

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

[dmazzoni, 2014-09-10 21:32:53]

dtseng: initial review before I send to OWNERS
xiyuan: FYI

Depends on: https://codereview.chromium.org/557393003/

[David Tseng, 2014-09-10 22:50:29]

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
File chrome/browser/accessibility/accessibility_extension_api.cc (right):

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
chrome/browser/accessibility/accessibility_extension_api.cc:28: #if
defined(OS_CHROMEOS)
Are we going to be able to support all platforms? If so, add a todo here. If
not, then we should chat about ChromeVox next functionality tradeoffs on
non-CROS.

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
chrome/browser/accessibility/accessibility_extension_api.cc:331: }
I'd check to make sure the auto-generated api bindings doesn't already do all
this for you in some |Parse| method. I know in an idl specified api, it does.

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
chrome/browser/accessibility/accessibility_extension_api.cc:335: #endif  //
defined(OS_CHROMEOS)
Should fail on other platforms.

https://codereview.chromium.org/560983002/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/accessibility_private.json (right):

https://codereview.chromium.org/560983002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/accessibility_private.json:252: "description":
"Array of rectangles to focus."
This sounds like there would be a focus action taken upon a page. Do we simply
want to substitute "focus" with "indicator" everywhere to reduce
confusion/overloading?

[dmazzoni, 2014-09-11 22:04:36]

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
File chrome/browser/accessibility/accessibility_extension_api.cc (right):

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
chrome/browser/accessibility/accessibility_extension_api.cc:28: #if
defined(OS_CHROMEOS)
On 2014/09/10 22:50:28, David Tseng wrote:
> Are we going to be able to support all platforms? If so, add a todo here. If
> not, then we should chat about ChromeVox next functionality tradeoffs on
> non-CROS.

Implementing this on other platforms is possible but nontrivial. As we discussed
offline, we're going to implement this on Chrome OS only for now while ChromeVox
Next is still under development. As we get closer to release we can decide
whether we want to port something similar or implement an alternative.

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
chrome/browser/accessibility/accessibility_extension_api.cc:331: }
On 2014/09/10 22:50:28, David Tseng wrote:
> I'd check to make sure the auto-generated api bindings doesn't already do all
> this for you in some |Parse| method. I know in an idl specified api, it does.

Sure. I filed http://crbug.com/413421 and I'm happy to start on that now in
parallel. I'll merge either way depending on what change happens first.

https://codereview.chromium.org/560983002/diff/1/chrome/browser/accessibility...
chrome/browser/accessibility/accessibility_extension_api.cc:335: #endif  //
defined(OS_CHROMEOS)
On 2014/09/10 22:50:28, David Tseng wrote:
> Should fail on other platforms.

Done.

https://codereview.chromium.org/560983002/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/accessibility_private.json (right):

https://codereview.chromium.org/560983002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/accessibility_private.json:252: "description":
"Array of rectangles to focus."
On 2014/09/10 22:50:28, David Tseng wrote:
> This sounds like there would be a focus action taken upon a page. Do we simply
> want to substitute "focus" with "indicator" everywhere to reduce
> confusion/overloading?

I clarified the text a bit.

I think it's clearer to call it an accessibility focus ring here rather than an
active indicator. The concept of focus and a focus ring is well-understood, it
makes it easier to understand and easier to read throughout the Chrome codebase.
Inside ChromeVox we use "active indicator" because we have to carefully track
keyboard focus and that's not always the same as the active element.

Another reason is that I intend to use the same code in Chrome OS to implement
support for a "highlight focused element" accessibility feature independent of
ChromeVox.

[dmazzoni, 2014-09-11 22:05:37]

dmazzoni@chromium.org changed reviewers:
+ isherman@chromium.org, rockot@chromium.org

[dmazzoni, 2014-09-11 22:05:38]

+rockot for chrome/common/extensions
+isherman for histogram

[Ken Rockot(use gerrit already), 2014-09-11 22:07:09]

extensions lgtm

[Ilya Sherman, 2014-09-12 01:31:30]

histograms lgtm

[dmazzoni, 2014-09-12 23:33:20]

The CQ bit was checked by dmazzoni@chromium.org

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

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/560983002/60001

[commit-bot: I haz the power, 2014-09-13 01:18:38]

Committed patchset #4 (id:60001) as c6625f0cd199e2d41e9b6e6e1ddd9ae4f4e44e0c

[commit-bot: I haz the power, 2014-09-13 01:21:15]

Patchset 4 (id:??) landed as
https://crrev.com/fecaedc03904c091897dea2926ec00756f6e53ed
Cr-Commit-Position: refs/heads/master@{#294719}