[Android] When toggling AX script inject, also set native AX state.

[Android] When toggling AX script inject, also set native AX state.

When we enable/disable script injection, also update the flag that
says whether we should use native accessibility.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=225491

[benm (inactive), 2013-09-25 11:08:53]

PTAL

[joth, 2013-09-25 11:41:28]

lgtm % small simplification

https://codereview.chromium.org/24559002/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/24559002/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2996:
mNativeAccessibilityAllowed = true;
bool use_chromevox = isDeviceAccessibilityScriptInjectionEnabled();

setInjectedAccessibility(use_chromevox);
mNativeAccessibilityAllowed = !use_chromevox;

[dmazzoni, 2013-09-25 16:07:05]

Note that setting mNativeAccessibilityAllowed to false won't actually disable it
if it was already enabled, is that okay?

It's not clear to me if there's a race condition.

[benm (inactive), 2013-09-25 17:32:42]

On 2013/09/25 16:07:05, Dominic Mazzoni wrote:
> Note that setting mNativeAccessibilityAllowed to false won't actually disable
it
> if it was already enabled, is that okay?

Good point, so I think we should also set
mNativeAccessibilityEnabled = false;
nativeSetAccessibilityEnabled(mNativeContentViewCore, false);

for the case that we're switching from native to ChromeVox.

> 
> It's not clear to me if there's a race condition.

I don't think so, I think we just need to make sure we remain consistent when we
choose one mode or the other.

[benm (inactive), 2013-09-25 18:10:27]

On 2013/09/25 17:32:42, benm wrote:
> On 2013/09/25 16:07:05, Dominic Mazzoni wrote:
> > Note that setting mNativeAccessibilityAllowed to false won't actually
disable
> it
> > if it was already enabled, is that okay?
> 
> Good point, so I think we should also set
> mNativeAccessibilityEnabled = false;
> nativeSetAccessibilityEnabled(mNativeContentViewCore, false);

Hmm, so calling nativeSetAccessibilityEnabled(false) in the case that we're
going from native -> Chrome Vox (e.g. the use case of enabling JS through a
browser's settings menu) doesn't seem to work - we enable both on page reload. I
see the ChromeVox highlight over the first DOM element, but touch to explore
uses native accessibility.

Switching from ChromeVox -> native doesn't work either (e.g. disabling JS in the
browsers menu and reloading). I don't get ChromeVox, but don't seem to get
native accessibility either ... :-/

[benm (inactive), 2013-09-25 18:48:07]

On 2013/09/25 18:10:27, benm wrote:
> On 2013/09/25 17:32:42, benm wrote:
> > On 2013/09/25 16:07:05, Dominic Mazzoni wrote:
> > > Note that setting mNativeAccessibilityAllowed to false won't actually
> disable
> > it
> > > if it was already enabled, is that okay?
> > 
> > Good point, so I think we should also set
> > mNativeAccessibilityEnabled = false;
> > nativeSetAccessibilityEnabled(mNativeContentViewCore, false);
> 
> Hmm, so calling nativeSetAccessibilityEnabled(false) in the case that we're
> going from native -> Chrome Vox (e.g. the use case of enabling JS through a
> browser's settings menu) doesn't seem to work - we enable both on page reload.
I
> see the ChromeVox highlight over the first DOM element, but touch to explore
> uses native accessibility.
> 
> Switching from ChromeVox -> native doesn't work either (e.g. disabling JS in
the
> browsers menu and reloading). I don't get ChromeVox, but don't seem to get
> native accessibility either ... :-/

Just to clarify - it's the toggling case that doesn't seem to do the right thing
with my patch applied - if the app is consistent within it's ContentViewCore
(either ChromeVox or native) then things work fine.

[dmazzoni, 2013-09-25 18:59:49]

Are you toggling using the TalkBack checkbox?

I'm not sure that supporting toggling that checkbox for a running app is an
important goal. (It's nice to have, but not important enough to merge.)

Are things working correctly now for the case where the WebView has javascript
disabled, like on that legal page?

[dmazzoni, 2013-09-25 19:45:56]

lgtm with the understanding that we want to follow up with a more complete fix
that handles all of the cases where javascript flips on/off or TalkBack's
preference flips on/off.

[commit-bot: I haz the power, 2013-09-26 09:35:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benm@chromium.org/24559002/8001

[commit-bot: I haz the power, 2013-09-26 17:36:15]

Change committed as 225491