Marcos Caceres [:marcosc] Reporter
	Description • 9 years ago

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.

	Marcos Caceres [:marcosc] Reporter
	Comment 1 • 9 years ago

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

	Tracy Walker [:tracy]
	Comment 2 • 8 years ago

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

	Marcos Caceres [:marcosc] Reporter
	Comment 3 • 8 years ago

(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.

	Marcos Caceres [:marcosc] Reporter
	Comment 4 • 8 years ago

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

	Jim Mathies [:jimm]
	Comment 5 • 8 years ago

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

	Jimmy Wang (:jimicy) - works on e10s stuff
	Comment 7 • 8 years ago

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.

	:Gijs (he/him)
	Comment 9 • 8 years ago

(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?

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 10 • 8 years ago

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

	Trevor Saunders (:tbsaunde)
	Comment 11 • 8 years ago

(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.

	alexander :surkov (:asurkov)
	Comment 12 • 8 years ago

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?

	Marco Zehe (:MarcoZ)
	Comment 13 • 8 years ago

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.

	Stephen A Pohl [:spohl]
	Comment 14 • 8 years ago

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.

	Stephen A Pohl [:spohl]
	Comment 15 • 8 years ago

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!

	alexander :surkov (:asurkov)
	Comment 16 • 8 years ago

(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.

	Jim Mathies [:jimm]
	Comment 17 • 8 years ago

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

	Stephen A Pohl [:spohl]
	Comment 18 • 8 years ago

(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.

	Pulsebot
	Comment 22 • 4 years ago

Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/92d97e70eb6b
Support AXSelectedTextMarkerRange. r=morgan