Various changes required to support ChromeVox Next to read Views and Windows.

Various changes required to support ChromeVox Next to read Views and Windows.

- adds support for valueChanged event (e.g. to read adjustment of the volume slider)
- add textChanged event handler to process omnibox character echo/deletion.
- braille support for the textChanged,textSelectionChanged events
- whitelist ChromeVox for automation API up to stable.
- remove the --enable-chromevox-next cmd line flag.
- adds options for Output module (to only output braille, for example).
- adds a "dummy" alert window as a child of the desktop root node; this window will be used to fire alert events. Due to the way we support alerts now without backing views, this is necessary.
- start generating context for window on "enter". This picks up announcements like "status tray" when entering the status tray bar with alt+shift+s.
- exclude the first child (only child) of the desktop node from having a name set; this is because that particular window has a non-human readable name "Display<xxx>" which gets read frequently when entering a screen/display.

Committed: https://crrev.com/113aa4f1f3e207b4b134ecc7316b9c46e29a22ad
Cr-Commit-Position: refs/heads/master@{#306855}

[David Tseng, 2014-11-20 19:47:07]

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

[David Tseng, 2014-11-20 19:48:25]

Ready for a look. Almost all views should now read properly with the new output
module except for treeitems. Working on that one still since most if not all
information needed can be computed from the tree itself.

[dmazzoni, 2014-11-20 21:37:20]

lgtm

Fantastic!!!

https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/chromevox.gyp (right):

https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:27:
'chromevox2_manifest',
It's a little confusing to reference chromevox2_manifest here since there's only
one now - want to rename it chromevox_manifest?

https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:159: 'target_name':
'chromevox1_manifest',
Maybe this one should be chromevox_webstore_manifest?

https://codereview.chromium.org/743273002/diff/40001/chrome/common/extensions...
File chrome/common/extensions/api/_manifest_features.json (right):

https://codereview.chromium.org/743273002/diff/40001/chrome/common/extensions...
chrome/common/extensions/api/_manifest_features.json:41: "channel": "stable",
Do we need something for channel: beta or is that implicit?

[Peter Lundblad, 2014-11-21 11:44:44]

Hi,

I think the change looks good, but I recommend splitting the functionali
changes for chromevox next out and make this cl only do the necessary
changes to 'flip the switch' to the new background page.  That'll make
reverting easier if we need to do that, perhaps after branch cut.

[chromium-reviews, 2014-11-24 18:10:54]

PTAL. Had to introduce a dummy alert window due to the way alerts were
handled in AccessibilityPrivate.

Also, the current set of spoken feedback tests are heavily tied to the
way current ChromeVox generates utterances and chunks them. What are
your thoughts on preserving these tests? I'll basically need to rewrite
them all.


dmazzoni@chromium.org writes:

> lgtm
>
> Fantastic!!!
>
>
>
https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
> File chrome/browser/resources/chromeos/chromevox/chromevox.gyp (right):
>
>
https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:27:
> 'chromevox2_manifest',
> It's a little confusing to reference chromevox2_manifest here since
> there's only one now - want to rename it chromevox_manifest?

Done (and for guest).
>
>
https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:159:
> 'target_name': 'chromevox1_manifest',
> Maybe this one should be chromevox_webstore_manifest?
>
Done.

>
https://codereview.chromium.org/743273002/diff/40001/chrome/common/extensions...
> File chrome/common/extensions/api/_manifest_features.json (right):
>
>
https://codereview.chromium.org/743273002/diff/40001/chrome/common/extensions...
> chrome/common/extensions/api/_manifest_features.json:41: "channel":
> "stable",
> Do we need something for channel: beta or is that implicit?

I think it's implicit. I'll let kalman comment.

>
> https://codereview.chromium.org/743273002/


-- 
Foo X. Bar
http://www.example.com

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

[chromium-reviews, 2014-11-24 18:16:25]

Great idea; I'll split the cl into background page move and the long
tail of things needed to get us going with ChromeVox Next in views.

I'm going to make this cl the misc changes for ChromeVox Next + views
since that's basically the bulk of the change and send out another to do
the actual flip. The flip currently depends on fixing the spoken
feedback tests, so may come a bit later.


plundblad@chromium.org writes:

> Hi,
>
> I think the change looks good, but I recommend splitting the functionali
> changes for chromevox next out and make this cl only do the necessary
> changes to 'flip the switch' to the new background page.  That'll make
> reverting easier if we need to do that, perhaps after branch cut.
>
>
>
> https://codereview.chromium.org/743273002/
>
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.


-- 
Foo X. Bar
http://www.example.com

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

[David Tseng, 2014-11-24 21:24:41]

On 2014/11/24 18:10:54, chromium-reviews wrote:
> PTAL. Had to introduce a dummy alert window due to the way alerts were
> handled in AccessibilityPrivate.
> 
> Also, the current set of spoken feedback tests are heavily tied to the
> way current ChromeVox generates utterances and chunks them. What are
> your thoughts on preserving these tests? I'll basically need to rewrite
> them all.
Disregard this; it turned out that there were a few changes needed required to
generating context for windows and a few other places where
SendAccessibilityControlNotification was being used directly without an actual
backing View.

All SpokenFeedbackTests appear to now pass locally; awaiting try bot results
now.

> 
> 
> mailto:dmazzoni@chromium.org writes:
> 
> > lgtm
> >
> > Fantastic!!!
> >
> >
> >
>
https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
> > File chrome/browser/resources/chromeos/chromevox/chromevox.gyp (right):
> >
> >
>
https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
> > chrome/browser/resources/chromeos/chromevox/chromevox.gyp:27:
> > 'chromevox2_manifest',
> > It's a little confusing to reference chromevox2_manifest here since
> > there's only one now - want to rename it chromevox_manifest?
> 
> Done (and for guest).
> >
> >
>
https://codereview.chromium.org/743273002/diff/40001/chrome/browser/resources...
> > chrome/browser/resources/chromeos/chromevox/chromevox.gyp:159:
> > 'target_name': 'chromevox1_manifest',
> > Maybe this one should be chromevox_webstore_manifest?
> >
> Done.
> 
> >
>
https://codereview.chromium.org/743273002/diff/40001/chrome/common/extensions...
> > File chrome/common/extensions/api/_manifest_features.json (right):
> >
> >
>
https://codereview.chromium.org/743273002/diff/40001/chrome/common/extensions...
> > chrome/common/extensions/api/_manifest_features.json:41: "channel":
> > "stable",
> > Do we need something for channel: beta or is that implicit?
> 
> I think it's implicit. I'll let kalman comment.
> 
> >
> > https://codereview.chromium.org/743273002/
> 
> 
> -- 
> Foo X. Bar
> http://www.example.com
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Peter Lundblad, 2014-11-25 10:19:59]

lgtm

[David Tseng, 2014-11-26 21:14:57]

dtseng@chromium.org changed reviewers:
+ kalman@chromium.org, sky@chromium.org

[David Tseng, 2014-11-26 21:14:57]

+ sky, kalman for OWNERS

[not at google - send to devlin, 2014-11-26 21:22:17]

extensions lgtm with 1 comment

https://codereview.chromium.org/743273002/diff/160001/chrome/common/extension...
File chrome/common/extensions/api/_manifest_features.json (right):

https://codereview.chromium.org/743273002/diff/160001/chrome/common/extension...
chrome/common/extensions/api/_manifest_features.json:42: "extension_types":
["extension", "legacy_packaged_app"],
Please file a bug for this whitelist and reference it here (so that the
extensions team can do easier accounting of our whitelists)

[not at google - send to devlin, 2014-11-26 21:22:47]

On 2014/11/26 21:22:17, kalman wrote:
> extensions lgtm with 1 comment
> 
>
https://codereview.chromium.org/743273002/diff/160001/chrome/common/extension...
> File chrome/common/extensions/api/_manifest_features.json (right):
> 
>
https://codereview.chromium.org/743273002/diff/160001/chrome/common/extension...
> chrome/common/extensions/api/_manifest_features.json:42: "extension_types":
> ["extension", "legacy_packaged_app"],
> Please file a bug for this whitelist and reference it here (so that the
> extensions team can do easier accounting of our whitelists)

Oh - there are instructions at the top of the file about how to add whitelists,
which includes bug filing procedure.

[sky, 2014-12-01 17:02:52]

What files do you need me to review?

[David Tseng, 2014-12-04 17:08:45]

dtseng@chromium.org changed reviewers:
+ zelidrag@chromium.org
- sky@chromium.org

[David Tseng, 2014-12-04 17:08:46]

- sky (sorry, added you because of git cl owners).
+ zelidrag for *chromeos*

[David Tseng, 2014-12-04 17:27:41]

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

[David Tseng, 2014-12-04 17:27:42]

+ oshima for chrome/browser/ui/ash/chrome_shell_delegate_chromeos.cc

[oshima, 2014-12-04 18:15:41]

lgtm

[David Tseng, 2014-12-04 18:20:40]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-12-04 18:21:30]

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

[commit-bot: I haz the power, 2014-12-04 18:35:05]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2014-12-04 18:35:56]

Patchset 13 (id:??) landed as
https://crrev.com/113aa4f1f3e207b4b134ecc7316b9c46e29a22ad
Cr-Commit-Position: refs/heads/master@{#306855}

[jonross, 2014-12-04 19:22:44]

A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/779103002/ by jonross@chromium.org.

The reason for reverting is: This change broken the ChromiumOS on Chromium
build:

http://build.chromium.org/p/chromiumos.chromium/builders/Daisy%20%28chromium%...

With the error:
chrome/browser/ui/ash/accessibility/ax_root_obj_wrapper.h:17:7: error:
'AXRootObjWrapper' has a field 'AXRootObjWrapper::alert_window_' whose type uses
the anonymous namespace [-Werror].