1019425 - When TalkBack is suspended, Firefox partially behaves as if it isn't

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Kostadin Kolev]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140506152807

Steps to reproduce:

1. Start TalkBack from Settings -> Accessibility -> TalkBack.
2. Open Firefox and load a page in it (no matter witch).
3. Suspend TalkBack via its global context menu (invoke it via the down then right gesture) -> Suspend Feedback.
4. Try to interact with Firefox, incl. scrolling and zooming the loaded page.


Actual results:

Zooming with 2 fingers doesn't work. Actually, zooming with 3 fingers I think doesn't work either, but that is a different bug. Scrolling is sluggish, as described in bug #1019420.


Expected results:

Zooming with 2 fingers should work. Scrolling should be smooth, as it is with no TalkBack enabled.

Comment 1

[Patrick H. Lauke]

Confirming that this is still an issue. Once you go into "pause feedback"/"suspended" mode, Firefox/Android still behaves as if it was in screen reader mode - you cannot simply tap on links etc to activate them (sometimes, they strangely fire up the "How do you want to open this link..." dialog, inexplicably); you can only swipe left/right to move the focus; but there's no audio output.

I made a video to illustrate this more clearly: https://www.youtube.com/watch?v=gqALS69fyhs

Comment 2

[Patrick H. Lauke]

Bumping this, as it's still a problem in latest Firefox 45.0.2 on Android 6.0.1

Comment 3

[Marco Zehe (:MarcoZ)]

Eitan, do you have any idea on this?

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: MozReview Request: Bug 1019425 - Activate accessibility when both accessibilty and explore by touch are enabled. r?mfinkle

This simplifies some old code. We get rid of a whitelist of services, and rely on the face that accessibility and explore by touch is enabled to enter our internal accessibility mode.

The whitelist methode held 2 assumptions that are not true anymore:

1. There are some non-accessibility accessibility service apps in the wild that read notifications.
In later Android versions this has been made into a non-a11y use case with NotificationListenerService. So the cases of non-a11y accessibility services has shrunk.

2. Not all screen readers (Gingerbread TalkBack, for example) supported explore by touch. Today, we
exclusively support accessibility services that use explore by touch, and we should not activate our
accessibility mode in any other case.

Review commit: https://reviewboard.mozilla.org/r/50077/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50077/

Comment 5

[Eitan Isaacson [:eeejay]]

The patch fixes this suspend bug because we now listen for the toggling of both accessibility and explore by touch (the latter is toggled when talkback goes into suspend mode).

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8747941 [details]
MozReview Request: Bug 1019425 - Activate accessibility when both accessibilty and explore by touch are enabled. r?mfinkle

https://reviewboard.mozilla.org/r/50077/#review46881

Looks OK, just take a look at the nits.

I assume you did the required testing.

::: mobile/android/base/java/org/mozilla/gecko/GeckoAccessibility.java:248
(Diff revision 1)
>              layerview.setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_YES);
>          }
>      }
>  
> -    public static void setAccessibilityStateChangeListener(final Context context) {
> +    public static void setAccessibilityManagerListeners(final Context context) {
>          // The state change listener is only supported on API14+

I assume you are removing this because we dropped support for Gingerbread and Honeycomb.

Did you want to update or remove the comment?

::: mobile/android/base/java/org/mozilla/gecko/GeckoAccessibility.java:263
(Diff revision 1)
> -            });
> +        });
> +
> +        accessibilityManager.addTouchExplorationStateChangeListener(new AccessibilityManager.TouchExplorationStateChangeListener() {
> +            @Override
> +            public void onTouchExplorationStateChanged(boolean enabled) {
> +                Log.i("eeejay", "onTouchExplorationStateChanged " + enabled);

Remove the debugging Log?

Comment 7

[Eitan Isaacson [:eeejay]]

https://reviewboard.mozilla.org/r/50077/#review46881

> I assume you are removing this because we dropped support for Gingerbread and Honeycomb.
> 
> Did you want to update or remove the comment?

Yes. I want to remove this stuff more aggressively in the future. Considering actually dropping ICS support as well since the a11y API so radically evolved in JB.

Comment 8

[Eitan Isaacson [:eeejay]]

Comment on attachment 8747941 [details]
MozReview Request: Bug 1019425 - Activate accessibility when both accessibilty and explore by touch are enabled. r?mfinkle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50077/diff/1-2/

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cdec576dad5e

Comment 10

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5a8498e90692 for breaking every test suite like https://treeherder.mozilla.org/logviewer.html#?job_id=27176662&repo=mozilla-inbound.

Comment 11

[Eitan Isaacson [:eeejay]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffbfb9d4b152

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f70bee0d12e

Comment 13

[Eitan Isaacson [:eeejay]]

Added a conditional for the part that is API 19+. Oops!!

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1f70bee0d12e