After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 520612 - Cursor routing key support in firefox
Cursor routing key support in firefox
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: braille
unspecified
Other Linux
: Normal enhancement
: 2.24.0
Assigned To: Eitan Isaacson
Orca Maintainers
Depends on: 523268
Blocks: 520613 521599
 
 
Reported: 2008-03-05 22:23 UTC by Eitan Isaacson
Modified: 2009-03-10 00:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed changes (14.29 KB, patch)
2008-03-06 23:56 UTC, Eitan Isaacson
needs-work Details | Review
Proposed patch (11.40 KB, patch)
2008-03-18 23:32 UTC, Eitan Isaacson
none Details | Review
Proposed patch (5.67 KB, patch)
2008-03-18 23:46 UTC, Eitan Isaacson
needs-work Details | Review
Proposed patch (5.98 KB, patch)
2008-03-19 19:43 UTC, Eitan Isaacson
needs-work Details | Review
Proposed patch (9.54 KB, patch)
2008-03-21 22:08 UTC, Eitan Isaacson
none Details | Review
Proposed patch (10.20 KB, patch)
2008-03-25 23:14 UTC, Eitan Isaacson
none Details | Review
Proposed patch (11.14 KB, patch)
2008-04-03 22:07 UTC, Eitan Isaacson
needs-work Details | Review
Proposed patch (12.09 KB, patch)
2008-04-08 20:55 UTC, Eitan Isaacson
committed Details | Review

Description Eitan Isaacson 2008-03-05 22:23:29 UTC
Firefox does not support cursor routing keys.
This is because document content is displayed with braille.Region, instead of braille.Text. braille.Region does not support cursor key routing.
Comment 1 Eitan Isaacson 2008-03-06 23:56:06 UTC
Created attachment 106725 [details] [review]
Proposed changes

This is the beginning of work that might work. I didn't test this thoroughly, but cursor key routing should work.
Comment 2 Willie Walker 2008-03-11 14:06:24 UTC
First coarse pass at GNOME 2.24 planning.
Comment 3 Eitan Isaacson 2008-03-18 23:32:18 UTC
Created attachment 107577 [details] [review]
Proposed patch

Added pydoc strings.
Comment 4 Eitan Isaacson 2008-03-18 23:46:40 UTC
Created attachment 107578 [details] [review]
Proposed patch

This patch depends on the patch in bug 523268.
I separated them so it's all a bit more clear.
Comment 5 Willie Walker 2008-03-19 16:19:05 UTC
Can you give me an idea of where this patch stands?  Is it still proof of concept?  One thing I noticed is that the gnome-terminal fix seems broken -- in looking at the patch, I don't see any return values from the new getTextLineAtCaret method.
Comment 6 Eitan Isaacson 2008-03-19 19:43:59 UTC
Created attachment 107644 [details] [review]
Proposed patch

Yes, this is still a proof of concept. This fixes the gnome-terminal issue.
I hope to come out with more functional patches before we actually consider committing this.
Comment 7 Eitan Isaacson 2008-03-21 22:08:00 UTC
Created attachment 107764 [details] [review]
Proposed patch

This patch adds support for flat review, this includes cursor routing, just like any other text area, so you could click on the character you want to go to from flat review.

It also supports link component regions, so if you click on the "Link" region in the braille display, the link will be activated.
Comment 8 Eitan Isaacson 2008-03-21 22:11:20 UTC
Mike, could you give this a spin and tell me what you think?

If you find a bug in your paypal account page, I will need your username and password :)
Comment 9 Mike Pedersen 2008-03-25 17:47:45 UTC
One thing I think we'd like to change here is to make a cursor routing press on a link or form control perform a mouse click.  This would for example allow for the clicking of links or activation of buttons.  
Comment 10 Eitan Isaacson 2008-03-25 18:45:03 UTC
That should actually work. I didn't want to enable it for clicks on the actual text, because the user might not know it is a link, and just want to move the caret. But if you lick on the "Link" text, it performs a click.

Do you think the former should be the behavior?

I didn't do the form control part, good idea.
Comment 11 Eitan Isaacson 2008-03-25 23:14:18 UTC
Created attachment 108031 [details] [review]
Proposed patch

This patch enables clicking on links, not just on the Link identifier, but the entire text. Also, forms seem to just work.
Comment 12 Eitan Isaacson 2008-04-03 22:07:45 UTC
Created attachment 108575 [details] [review]
Proposed patch

This has some changes that are a bit less intrusive.
Comment 13 Eitan Isaacson 2008-04-03 22:18:54 UTC
Changes committed to trunk. Unless something horribly wrong happened because of this patch, please file a new bug if there are more issues.
Comment 14 Joanmarie Diggs (IRC: joanie) 2008-04-04 04:32:16 UTC
This isn't "horribly wrong" but your patch introduces a regression (as discovered via the regression tests).  We are now displaying certain embedded object characters that we weren't before.  For example, navigate by line or by tabbing on the anchors2.html test file.

I'm not sure if your change exposed something bad/wrong in the way we were doing things or not.  One way or another we need to fix it.  I'm tired and will investigate tomorrow sometime, unless you beat me to it. :-)
Comment 15 Joanmarie Diggs (IRC: joanie) 2008-04-04 04:59:19 UTC
A couple of other symptoms/regressions found via the regression tests which need to be examined:

Cursor at the end of the list rather than the beginning of the list, for example:

Test 3 of 16 FAILED: /home/jd/orca/test/keystrokes/firefox/html_role_lists.py:Line Down
EXPECTED:
     "BRAILLE LINE:  '1. remember what the heck we are doing each day'",
     "     VISIBLE:  '1. remember what the heck we are', cursor=1",
     "SPEECH OUTPUT: '1. remember what the heck we are doing each day'",
ACTUAL:
     "BRAILLE LINE:  '1. remember what the heck we are doing each day'",
     "     VISIBLE:  ' the heck we are doing each day', cursor=32",
     "SPEECH OUTPUT: '1. remember what the heck we are doing each day'",


Missing characters (e.g. the 'r' in 'rather' and the ':' at the end of 'here'):

Test 41 of 41 FAILED: /home/jd/orca/test/keystrokes/firefox/label_guess_entries.py:Next form field
EXPECTED:
     "BRAILLE LINE:  'Type something Link  rather amusing Link  here:  $l'",
     "     VISIBLE:  ' $l', cursor=1",
     "SPEECH OUTPUT: 'Wrapping to top.'",
     "SPEECH OUTPUT: 'Type something rather amusing here: text'",
ACTUAL:
     "BRAILLE LINE:  'Type something Link  rathe amusing Link  here  $l'",
     "     VISIBLE:  ' $l', cursor=1",
     "SPEECH OUTPUT: 'Wrapping to top.'",
     "SPEECH OUTPUT: 'Type something rather amusing here: text'",
Comment 16 Eitan Isaacson 2008-04-04 21:24:49 UTC
I'm going to revert this patch for the time-being.
I knew it would show problems, I just didn't realize how fast.
Comment 17 Eitan Isaacson 2008-04-04 21:33:29 UTC
I am chasing down a lot of fundamental stuff. So I don't think I am going to fix this in trunk. Rather I will improve the patch here.
Sorry about all that.
Comment 18 Eitan Isaacson 2008-04-08 20:55:56 UTC
Created attachment 108892 [details] [review]
Proposed patch

Ok, the changes here should fix the regressions joanie mentioned above.
Joanie, could you give this a round of tests?

The firefox tests directory always stops working at some point, I think it's all those dojo tests. Do you run all of them?
Comment 19 Joanmarie Diggs (IRC: joanie) 2008-04-15 21:54:14 UTC
Eitan, I'm really sorry I didn't see/notice this comment when you asked it. :-(

We discussed this at team meeting, but for the sake of documentation:  The tests do not *normally* stop working at some point, but every once in a while I find that a test does cause the suite to get hung up. Running them this afternoon, it got hung up on dojo_checkbox.py.  When this happens, I give focus to the terminal window where I launched the tests and press Control C just once.  That seems to kill the current test without killing the suite.  And, sure enough, that worked today. :-)

Anyhoo....

Looking at the tests, there are quite a few "regressions" (note the quotation marks) that are arguably fixes because they are the removal of unnecessary and unwanted whitespace.  (Yea!!) A few examples:

~~~~~~~~~~~~~~~

- BRAILLE LINE:  'Foo Link '
?                         -

+ BRAILLE LINE:  'Foo Link'

~~~~~~~~~~~~~~~

- BRAILLE LINE:  'I know how to cope with misfortune, '
?                                                    -

+ BRAILLE LINE:  'I know how to cope with misfortune,'

~~~~~~~~~~~~~~~

- BRAILLE LINE:  ' $l and Now $l  '
?                               --

+ BRAILLE LINE:  ' $l and Now $l'

~~~~~~~~~~~~~~~

-      VISIBLE:  '  < > CheckBox  and remember th', cursor=1
?                 ---            -

+      VISIBLE:  '< > CheckBox and remember these ', cursor=1
?                                             ++++

~~~~~~~~~~~~~~~

I don't see any true regressions (i.e. in the breakage sense of the word). :-) 

Therefore, if this works in terms of cursor routing, my vote would be to get this patch checked in.  Once that has occurred, I'll update the regression tests to reflect the removed whitespace.
Comment 20 Eitan Isaacson 2008-04-15 23:36:37 UTC
Thanks for that Joanie!
I committed this to trunk.
Closing bug. If you think these changes are the culprit of future regressions,
please open a new one. Tanks!