1226473 - Firefox does not read selected text

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[Marcos Caceres [:marcosc]]

Screen reader support seems to have completely stopped working in Firefox in El Capitan. 

Steps:

1. select any text on a page. 
2. press Apple's key combo to read text (alt-esc) for me.

Expected:
Text to be read. 

Actual: 
Nothing is read. 


The only time it seems to work is in text areas. But doesn't work on any other text.

Comment 1

[Marcos Caceres [:marcosc]]

Actually, sometimes it just starts reading the whole tab instead of the selected text.

Comment 2

[Tracy Walker [:tracy]]

Marcos, does this work with e10s disabled? In Nightly > Preferences > General > Startup > (uncheck) Enable multi-process Nightly

Comment 3

[Marcos Caceres [:marcosc]]

(In reply to Tracy Walker [:tracy] from comment #2)
> Marcos, does this work with e10s disabled? In Nightly > Preferences >
> General > Startup > (uncheck) Enable multi-process Nightly

Yes. It seems to work fine without e10s enabled.

Comment 4

[Marcos Caceres [:marcosc]]

Actually, it only works for a little while... then I just breaks again and goes back to reading the whole page.

Comment 5

[Jim Mathies [:jimm]]

e10s is the new standard so we really need to fix this there.

Comment 7

[Jimmy Wang (:jimicy) - works on e10s stuff]

This is not a duplicate of bug 1261299.
This problem occurs in non-e10s as well.

I'm not familiar with our osx speech synthesis code.

The first text selection when you press (alt-esc) works. But if you press it again or make a new selection and press (alt-esc). You got these logs
> AXGroup can't respond to attribute AXSelectedText
> AXGroup can't respond to attribute AXSelectedTextMarkerRange

This bug should depend on bug 1261299 because it caches the current selection as a transferable, which this can use.

Comment 9

[:Gijs (he/him)]

(In reply to Jimmy Wang (:jimicy) - works on e10s stuff from comment #7)
> This is not a duplicate of bug 1261299.
> This problem occurs in non-e10s as well.
> 
> I'm not familiar with our osx speech synthesis code.
> 
> The first text selection when you press (alt-esc) works. But if you press it
> again or make a new selection and press (alt-esc). You got these logs
> > AXGroup can't respond to attribute AXSelectedText
> > AXGroup can't respond to attribute AXSelectedTextMarkerRange

These are from the mac Accessible implementation ( http://searchfox.org/mozilla-central/rev/03b3c20a6ec60435feb995a2a23c5926188c85a1/accessible/mac/mozAccessible.mm#439 ). Yura, do you know more about that and/or do you know someone who does?

Comment 10

[Yura Zenevich [:yzen] (please use "needinfo")]

I'm not familiar with this code, perhaps Alex would know?

Comment 11

[Trevor Saunders (:tbsaunde)]

(In reply to :Gijs Kruitbosch (PTO recovery mode) from comment #9)
> (In reply to Jimmy Wang (:jimicy) - works on e10s stuff from comment #7)
> > This is not a duplicate of bug 1261299.
> > This problem occurs in non-e10s as well.
> > 
> > I'm not familiar with our osx speech synthesis code.
> > 
> > The first text selection when you press (alt-esc) works. But if you press it
> > again or make a new selection and press (alt-esc). You got these logs
> > > AXGroup can't respond to attribute AXSelectedText
> > > AXGroup can't respond to attribute AXSelectedTextMarkerRange
> 
> These are from the mac Accessible implementation (
> http://searchfox.org/mozilla-central/rev/
> 03b3c20a6ec60435feb995a2a23c5926188c85a1/accessible/mac/mozAccessible.mm#439
> ). Yura, do you know more about that and/or do you know someone who does?

given that implementation of the function doesn't handle that attribute it seems like we are creating the wrong sub class of mozAccessible, but other than that somebody just needs to debug it.

Comment 12

[alexander :surkov (:asurkov)]

It's interesting but it seems we implement neither AXSelectedText nor AXSelectedTextMarkerRange. Steven fixed a similar bug a while ago a different way, bug 674612, maybe VoiceOver expectations were changed.

Marco, do you have ideas?

Comment 13

[Marco Zehe (:MarcoZ)]

Unfortunately I do not. VoiceOver was never involved in the bug Steven fixed. That was the Edit/Speak Selected Text feature, which bypasses VoiceOver and takes text from the app and sends it to the speech system directly. That's a different OS X function that has nothing to do with VoiceOver.

Comment 14

[Stephen A Pohl [:spohl]]

I have a patch that fixes this, but I want to figure out why it works and if it has side-effects before requesting review.

Comment 15

[Stephen A Pohl [:spohl]]

Attachment: Patch

The reason why VoiceOver is able to read the selected text the first time is because |accessibleParent| is NULL the first time and we call |GetNativeFromGeckoAccessible(accWrap->RootAccessible())| instead of |GetNativeFromGeckoAccessible(accessibleParent);|. Every subsequent attempt of making VoiceOver read the selected text fails because |accessibleParent| points to a valid object after the first time. We seem to be infinitely looping around when this is the case, which appears to be parent to child and back again (but I haven't been able to confirm for sure yet. Too much console spam).

This patch fixes the issue and VoiceOver is able to successfully read the selected text every time. We also don't have any infinite loops anymore.

Lorien, I don't know enough about this code to know if this is a valid fix, but I'm hoping that by looking at the patch, you could see what might be going on and how to fix it properly. Thanks!

Comment 16

[alexander :surkov (:asurkov)]

(In reply to Stephen A Pohl [:spohl] (OOO 10/13-10/20) from comment #15)
> Created attachment 8798139 [details] [diff] [review]
> Patch
> 
> The reason why VoiceOver is able to read the selected text the first time is
> because |accessibleParent| is NULL the first time and we call
> |GetNativeFromGeckoAccessible(accWrap->RootAccessible())| instead of
> |GetNativeFromGeckoAccessible(accessibleParent);|. Every subsequent attempt
> of making VoiceOver read the selected text fails because |accessibleParent|
> points to a valid object after the first time. We seem to be infinitely
> looping around when this is the case, which appears to be parent to child
> and back again (but I haven't been able to confirm for sure yet. Too much
> console spam).

afaik the parent is supposed to return immediate parent, not the root object. I'm not sure I've caught your correctly about looping, do we ever hang? otherwise how the loop gets broken?

> This patch fixes the issue and VoiceOver is able to successfully read the
> selected text every time. We also don't have any infinite loops anymore.

how does selected text is related with root object as a parent?

> Lorien, I don't know enough about this code to know if this is a valid fix,
> but I'm hoping that by looking at the patch, you could see what might be
> going on and how to fix it properly. Thanks!

Lorien is not around these days, you can ni? me instead, I don't know much about mac a11y part, but I'll do my best.

Comment 17

[Jim Mathies [:jimm]]

Looks like this is an a11y related bug vs. widget so moving this over.

Comment 18

[Stephen A Pohl [:spohl]]

(In reply to alexander :surkov from comment #16)
> (In reply to Stephen A Pohl [:spohl] (OOO 10/13-10/20) from comment #15)
> > Created attachment 8798139 [details] [diff] [review]
> > Patch
> > 
> > The reason why VoiceOver is able to read the selected text the first time is
> > because |accessibleParent| is NULL the first time and we call
> > |GetNativeFromGeckoAccessible(accWrap->RootAccessible())| instead of
> > |GetNativeFromGeckoAccessible(accessibleParent);|. Every subsequent attempt
> > of making VoiceOver read the selected text fails because |accessibleParent|
> > points to a valid object after the first time. We seem to be infinitely
> > looping around when this is the case, which appears to be parent to child
> > and back again (but I haven't been able to confirm for sure yet. Too much
> > console spam).
> 
> afaik the parent is supposed to return immediate parent, not the root
> object. I'm not sure I've caught your correctly about looping, do we ever
> hang? otherwise how the loop gets broken?

I'm not familiar with this code and I was just stating my observations.

> > This patch fixes the issue and VoiceOver is able to successfully read the
> > selected text every time. We also don't have any infinite loops anymore.
> 
> how does selected text is related with root object as a parent?

I was simply pointing out that this patch appears to fix the issue, which may help in diagnosing and fixing the problem correctly.

> > Lorien, I don't know enough about this code to know if this is a valid fix,
> > but I'm hoping that by looking at the patch, you could see what might be
> > going on and how to fix it properly. Thanks!
> 
> Lorien is not around these days, you can ni? me instead, I don't know much
> about mac a11y part, but I'll do my best.

It seems like someone with greater familiarity with a11y should take this over.

Comment 21

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1226473 - Support AXSelectedTextMarkerRange. r?morgan

To do this well we need to cache the text selection in the top level process.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92d97e70eb6b
Support AXSelectedTextMarkerRange. r=morgan

Comment 23

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/92d97e70eb6b