860013 - navigation by words broken with orca

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Trevor Saunders (:tbsaunde)]

str try to move through the text in the test case to be attached with control left / right to move by words, note that orca repeats the word it started on and doesn't move to the next / prev word.

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: test case

Comment 2

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #0)
> str try to move through the text in the test case to be attached with
> control left / right to move by words, note that orca repeats the word it
> started on and doesn't move to the next / prev word.

I'm sure our expectations about getTextAt/Before offsets are different. We need Orca to confirm we are correct (wrong). 

Joanie, do you have a chance to look at it please?

Comment 3

[Joanmarie Diggs]

At this moment, I don't have time. I'll add it to the list. In the meantime there are a couple of possibilities:

1. Your implementation is not jiving with what other implementations do. To test this theory, you could compare the AtkText behavior between Gecko and Gedit.

2. One of the many (and there are many) hacks in Orca to work around Gecko a11y bugs, differences in behavior, etc. might have become "broken" by a fix to Gecko.

Hope this is of some use in the meantime.

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Joanmarie Diggs from comment #3)
> At this moment, I don't have time. I'll add it to the list. In the meantime
> there are a couple of possibilities:
> 
> 1. Your implementation is not jiving with what other implementations do. To
> test this theory, you could compare the AtkText behavior between Gecko and
> Gedit.

Trev, can you compare please?

> 2. One of the many (and there are many) hacks in Orca to work around Gecko
> a11y bugs, differences in behavior, etc. might have become "broken" by a fix
> to Gecko.

Right, I considered this possibility

> Hope this is of some use in the meantime.

sure, thank you

Comment 5

[alexander :surkov (:asurkov)]

Attachment: accerciser output

the difference is in TEXT_BOUNDARY_WORD_END handling (it looks like there's something wrong in algorithm)

Comment 6

[alexander :surkov (:asurkov)]

I guess the problem is we return ('', 0, 0) for 0 offset so Orca gets in loop.
gedit returns ('a', 0, 1) for 0 offset (which is wrong btw).

ATK spec is contradictory (https://developer.gnome.org/atk/stable/AtkText.html#atk-text-get-text-at-offset):

"If the boundary_type is ATK_TEXT_BOUNDARY_WORD_END the returned string is from the word end before the offset to the word end at or after the offset."

There's no word end offset *before* 0 offset so ('', 0, 0) seems a correct value.

On the another hand the ATK spec states:

"The returned string will contain the word at the offset if the offset is inside a word and will contain the word after to the offset if the offset is not inside a word. "

which means that we should return ('a ', 0, 2) because 0 offset is inside a word 'a'.

Comment 7

[alexander :surkov (:asurkov)]

I filed a bug under ATK: https://bugzilla.gnome.org/show_bug.cgi?id=697779. If it takes long then I'd suggest to proceed with ('a ', 0, 2) as expected result. I think it should help.

Comment 8

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #6)

> "The returned string will contain the word at the offset if the offset is
> inside a word and will contain the word after to the offset if the offset is
> not inside a word. "
> 
> which means that we should return ('a ', 0, 2) because 0 offset is inside a
> word 'a'.

correction: should be ('a', 0, 1) since 1 is the word end of 'a' (gedit makes it right)

Comment 9

[alexander :surkov (:asurkov)]

Attachment: part1: another treatment of the spec

following second sentences of ATK spec (instead first sentences). It seems reasonable and it should help Orca to not halt at 0 offset. However Orca might freeze on a next word (because returned range is inclusive, i.e. [a, b] instead [a, b)). It'd better to check.

try server build will be here: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-67ada5cb0771

Comment 10

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #9)
> Created attachment 736300 [details] [diff] [review]
> patch
> 
> following second sentences of ATK spec (instead first sentences). It seems
> reasonable and it should help Orca to not halt at 0 offset. However Orca
> might freeze on a next word (because returned range is inclusive, i.e. [a,
> b] instead [a, b)). It'd better to check.
> 
> try server build will be here:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.
> [e-mail]-67ada5cb0771

sorry I haven't been able to test this before, I tried with a local build today and this didn't seem to fix things, but I'll try  the try build and a newer orca rsn.

Comment 11

[alexander :surkov (:asurkov)]

Maybe Orca expects gedit behavior, i.e. end word offsets return a next word (for example, 1 offset in 'a paragraph' returns ' paragraph' rather than 'a'). It doesn't go with the spec but actually we had the same behavior.

Joanie suggested ATK simplification and she wanted to fix Orca (https://mail.gnome.org/archives/gnome-accessibility-devel/2013-April/msg00000.html). Maybe we just should follow the spec regardless Orca brokeness.

Also we might to get back the behavior for Aurora to be on safe side.

Comment 12

[Joanmarie Diggs]

(In reply to alexander :surkov from comment #11)

> Joanie suggested ATK simplification and she wanted to fix Orca
> (https://mail.gnome.org/archives/gnome-accessibility-devel/2013-April/
> msg00000.html).

Simplifying the spec would remove things like getTextBeforeOffset(), getTextAfterOffset(), and the Start+End boundaries. The new boundaries would very likely have the behavior of the current "start" boundaries.

"What Gedit does" is really "what Gtk does." That behavior is also what WebKitGtk does. And ***that behavior is what Orca expects.*** So the fix to the spec might be to reword/clarify the spec to describe the expected behavior more clearly.

The fix to Orca would be ripping out all the Gecko-specific hacks. But doing that assumes that Orca has far fewer lines to piece together. Until that happens, ripping out all the hacks would only break things more.

> Maybe we just should follow the spec regardless Orca
> brokeness.

<sighs>

Comment 13

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #11)
> Maybe Orca expects gedit behavior, i.e. end word offsets return a next word
> (for example, 1 offset in 'a paragraph' returns ' paragraph' rather than
> 'a'). It doesn't go with the spec but actually we had the same behavior.

seems reasonable.  Have you filed a bug on gedit or otherwise tried to sort out why gedit is doing what you believe to be the wrong thing?

> Joanie suggested ATK simplification and she wanted to fix Orca
> (https://mail.gnome.org/archives/gnome-accessibility-devel/2013-April/
> msg00000.html). Maybe we just should follow the spec regardless Orca
> brokeness.

I saw, and yeah maybe, even though it makes my life kind of anoying :(  It sucks you won't be able to use old orca with new ff or other way around probably but I can't think of a better approach.

> Also we might to get back the behavior for Aurora to be on safe side.

ok, that might be reasonable thinking that our implementation will be better in more cases in this release.

Comment 14

[Joanmarie Diggs]

(In reply to Trevor Saunders (:tbsaunde) from comment #13)

> I saw, and yeah maybe, even though it makes my life kind of anoying :(  It
> sucks you won't be able to use old orca with new ff or other way around
> probably but I can't think of a better approach.

If you don't break the current Orca, you can then use the old Orca and the new Orca with the old Firefox and the new Firefox. :)

As I stated in my most recent comment and as I stated on the mailing list, I am not proposing to change Orca's behavior per se. I am proposing to keep the same *intended* behavior while removing hacks that are in Orca's Gecko-specific support to work around Gecko-specific bugs and other Gecko inconsistencies. But I cannot do that until there are other changes and fixes made in Gecko, including but not limited to, exposing linked text as links.

A long way of saying, I think the best approach is to please not break Orca.

Comment 15

[Joanmarie Diggs]

s/exposing linked text as links/exposing linked text as text/

Sorry, frustration-related typo.

Comment 16

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #13)
> (In reply to alexander :surkov from comment #11)
> > Maybe Orca expects gedit behavior, i.e. end word offsets return a next word
> > (for example, 1 offset in 'a paragraph' returns ' paragraph' rather than
> > 'a'). It doesn't go with the spec but actually we had the same behavior.
> 
> seems reasonable.  Have you filed a bug on gedit or otherwise tried to sort
> out why gedit is doing what you believe to be the wrong thing?

as Joanie said WebKitGtk :) No, I didn't. Probably it doesn't make huge sense if ATK text simplification will be done in nearest future.

> > Joanie suggested ATK simplification and she wanted to fix Orca
> > (https://mail.gnome.org/archives/gnome-accessibility-devel/2013-April/
> > msg00000.html). Maybe we just should follow the spec regardless Orca
> > brokeness.
> 
> I saw, and yeah maybe, even though it makes my life kind of anoying :(  It
> sucks you won't be able to use old orca with new ff or other way around
> probably but I can't think of a better approach.

We need to get synchronized in versions (like FF release + Orca release), otherwise nobody should really care.

> > Also we might to get back the behavior for Aurora to be on safe side.
> 
> ok, that might be reasonable thinking that our implementation will be better
> in more cases in this release.

Basically if ATK is going to be simplified then I don't care if we don't follow the previous spec version if we it fixes Orca.

Comment 17

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #16)
> (In reply to Trevor Saunders (:tbsaunde) from comment #13)
> > (In reply to alexander :surkov from comment #11)
> > > Maybe Orca expects gedit behavior, i.e. end word offsets return a next word
> > > (for example, 1 offset in 'a paragraph' returns ' paragraph' rather than
> > > 'a'). It doesn't go with the spec but actually we had the same behavior.
> > 
> > seems reasonable.  Have you filed a bug on gedit or otherwise tried to sort
> > out why gedit is doing what you believe to be the wrong thing?
> 
> as Joanie said WebKitGtk :) No, I didn't. Probably it doesn't make huge
> sense if ATK text simplification will be done in nearest future.

yeah, if we don't care about the spec I don't think it makes sense to try and make other people care.

> > > Joanie suggested ATK simplification and she wanted to fix Orca
> > > (https://mail.gnome.org/archives/gnome-accessibility-devel/2013-April/
> > > msg00000.html). Maybe we just should follow the spec regardless Orca
> > > brokeness.
> > 
> > I saw, and yeah maybe, even though it makes my life kind of anoying :(  It
> > sucks you won't be able to use old orca with new ff or other way around
> > probably but I can't think of a better approach.
> 
> We need to get synchronized in versions (like FF release + Orca release),
> otherwise nobody should really care.

not sure what you mean here.

> > > Also we might to get back the behavior for Aurora to be on safe side.
> > 
> > ok, that might be reasonable thinking that our implementation will be better
> > in more cases in this release.
> 
> Basically if ATK is going to be simplified then I don't care if we don't
> follow the previous spec version if we it fixes Orca.

ok, that's fine with me, but it seems if we don't care about old spec then we need to have idea what we are implementing wrtten down somewhere.

(In reply to Joanmarie Diggs from comment #14)
> (In reply to Trevor Saunders (:tbsaunde) from comment #13)
> 
> > I saw, and yeah maybe, even though it makes my life kind of anoying :(  It
> > sucks you won't be able to use old orca with new ff or other way around
> > probably but I can't think of a better approach.
> 
> If you don't break the current Orca, you can then use the old Orca and the
> new Orca with the old Firefox and the new Firefox. :)

sure if we do that then gecko is fine, but what about other people who took the old spec at its word.

> As I stated in my most recent comment and as I stated on the mailing list, I
> am not proposing to change Orca's behavior per se. I am proposing to keep
> the same *intended* behavior while removing hacks that are in Orca's
> Gecko-specific support to work around Gecko-specific bugs and other Gecko
> inconsistencies. But I cannot do that until there are other changes and
> fixes made in Gecko, including but not limited to, exposing linked text as
> links.

and I still don't understand why Jamie is wrong and you won't still need that code for things other than links.

Comment 18

[Trevor Saunders (:tbsaunde)]

> > Gecko-specific support to work around Gecko-specific bugs and other Gecko
> > inconsistencies. But I cannot do that until there are other changes and
> > fixes made in Gecko, including but not limited to, exposing linked text as
> > links.
> 
> and I still don't understand why Jamie is wrong and you won't still need
> that code for things other than links.

now maybe I should just shrug and accept it somehow magically will help, but I think it would be better and not just for us (libre / open office come to mind) if atk and ia2 text worked the same.

Comment 19

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #17)

> > > I saw, and yeah maybe, even though it makes my life kind of anoying :(  It
> > > sucks you won't be able to use old orca with new ff or other way around
> > > probably but I can't think of a better approach.
> > 
> > We need to get synchronized in versions (like FF release + Orca release),
> > otherwise nobody should really care.
> 
> not sure what you mean here.

I meant we should avoid the case when new Firefox was released but only old Orca is available and vise versa.  

> > > > Also we might to get back the behavior for Aurora to be on safe side.
> > > 
> > > ok, that might be reasonable thinking that our implementation will be better
> > > in more cases in this release.
> > 
> > Basically if ATK is going to be simplified then I don't care if we don't
> > follow the previous spec version if we it fixes Orca.
> 
> ok, that's fine with me, but it seems if we don't care about old spec then
> we need to have idea what we are implementing wrtten down somewhere.

I blog regularly about changes we do. Not sure what else we can do until we raise up our docs from ruins.

Comment 20

[alexander :surkov (:asurkov)]

Attachment: patch: a workaround

do as WebKitGtk does (ignore spec)

Comment 21

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #20)
> Created attachment 737831 [details] [diff] [review]
> patch: a workaround
> 
> do as WebKitGtk does (ignore spec)

this one plus original patch: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-4e29a6661259

I hope this helps

Comment 22

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #21)
> (In reply to alexander :surkov from comment #20)
> > Created attachment 737831 [details] [diff] [review]
> > patch: a workaround
> > 
> > do as WebKitGtk does (ignore spec)
> 
> this one plus original patch:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.
> [e-mail]-4e29a6661259
> 
> I hope this helps

can you do linux64 instead of linux please?

Comment 23

[alexander :surkov (:asurkov)]

it'll be here http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-88c3838f2f6a

Comment 24

[alexander :surkov (:asurkov)]

Trev, ping

Comment 25

[Trevor Saunders (:tbsaunde)]

ok, that try build seems to work in the bit of testing I did.  Now what am I supposed to review?  And what is that supposed to do?

Comment 26

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #25)
> ok, that try build seems to work in the bit of testing I did.  Now what am I
> supposed to review?  And what is that supposed to do?

you need to review "patch" patch (a first part of the bug). I will file a clean version of "patch: a workaround" (a second part of the bug).

Comment 27

[alexander :surkov (:asurkov)]

Comment on attachment 736300 [details] [diff] [review]
part1: another treatment of the spec

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b2020da6f66

Comment 28

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3b2020da6f66

Comment 29

[alexander :surkov (:asurkov)]

Attachment: part2: mimic WebKitGtk

Comment 30

[Trevor Saunders (:tbsaunde)]

Comment on attachment 745528 [details] [diff] [review]
part2: mimic WebKitGtk

more a rubber stamp than anything actually useful, but r=me

Comment 31

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #30)
> Comment on attachment 745528 [details] [diff] [review]
> part2: mimic WebKitGtk
> 
> more a rubber stamp than anything actually useful, but r=me

http://hg.mozilla.org/integration/mozilla-inbound/rev/1d57bd69b662

Comment 32

[alexander :surkov (:asurkov)]

Trev, please try a nightly after merging, we should backport it to Firefox 22

Comment 33

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

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

Comment 34

[alexander :surkov (:asurkov)]

Comment on attachment 736300 [details] [diff] [review]
part1: another treatment of the spec

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 853340
User impact if declined: Orca can't read successfully the web pages
Testing completed (on m-c, etc.): mochitest, manual testing
Risk to taking this patch (and alternatives if risky): rolling back to previous behavior, risk is small
String or IDL/UUID changes made by this patch: no

Comment 35

[alexander :surkov (:asurkov)]

Comment on attachment 745528 [details] [diff] [review]
part2: mimic WebKitGtk

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 853340
User impact if declined: Orca can't read successfully the web pages
Testing completed (on m-c, etc.): mochitest, manual testing
Risk to taking this patch (and alternatives if risky): rolling back to previous behavior, risk is small
String or IDL/UUID changes made by this patch: no

Comment 36

[Alex Keybl [:akeybl]]

Comment on attachment 736300 [details] [diff] [review]
part1: another treatment of the spec

Approving for Beta 22.

Comment 37

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ac3467b8e69c
https://hg.mozilla.org/releases/mozilla-beta/rev/9d0d2d5e6497