navigation by words broken with orca

RESOLVED FIXED in Firefox 22

Status

()

defect
--
major
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: tbsaunde, Assigned: surkov)

Tracking

(Blocks 1 bug, {regression})

unspecified
mozilla23
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox22 fixed, firefox23 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

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.
Posted file test case
(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?
Flags: needinfo?(jdiggs)
Blocks: getText*a11y
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.
Flags: needinfo?(jdiggs)
(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
Posted file accerciser output
the difference is in TEXT_BOUNDARY_WORD_END handling (it looks like there's something wrong in algorithm)
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'.
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.
(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)
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.alexander@gmail.com-67ada5cb0771
Assignee: nobody → surkov.alexander
Attachment #736300 - Flags: review?(trev.saunders)
(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.
> alexander@gmail.com-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.
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.
(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>
(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.
(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.
s/exposing linked text as links/exposing linked text as text/

Sorry, frustration-related typo.
(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.
(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.
> > 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.
(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.
Posted patch patch: a workaround (obsolete) — Splinter Review
do as WebKitGtk does (ignore spec)
(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.alexander@gmail.com-4e29a6661259

I hope this helps
(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.
> alexander@gmail.com-4e29a6661259
> 
> I hope this helps

can you do linux64 instead of linux please?
Trev, ping
Severity: normal → major
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?
(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).
Attachment #736300 - Flags: review?(trev.saunders) → review+
Whiteboard: [leave open]
Attachment #737831 - Attachment is obsolete: true
Attachment #745528 - Flags: review?(trev.saunders)
Attachment #736300 - Attachment description: patch → part1: another treatment of the spec
Comment on attachment 745528 [details] [diff] [review]
part2: mimic WebKitGtk

more a rubber stamp than anything actually useful, but r=me
Attachment #745528 - Flags: review?(trev.saunders) → review+
(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
Whiteboard: [leave open]
Trev, please try a nightly after merging, we should backport it to Firefox 22
https://hg.mozilla.org/mozilla-central/rev/1d57bd69b662
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
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
Attachment #736300 - Flags: approval-mozilla-aurora?
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
Attachment #745528 - Flags: approval-mozilla-aurora?
Comment on attachment 736300 [details] [diff] [review]
part1: another treatment of the spec

Approving for Beta 22.
Attachment #736300 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
Attachment #745528 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.