GNOME Bugzilla – Bug 535023
Structural Navigation should be pulled out of Gecko and include more objects
Last modified: 2009-03-10 00:05:18 UTC
1. Currently in Orca, structural navigation of documents is limited to Firefox and Thunderbird. It would be nice if other scripts (e.g. the script for StarOffice/OpenOffice) could take advantage of this functionality. 2. Pulling this functionality out of Gecko would make Gecko/script.py a heck of a lot shorter. :-) 3. The Orca community has grown considerably over the past year. Questions such as "What's the command to jump to the next entry/combo box/button/etc.?" are being raised with (what strikes me to be) increased frequency. It would be nice to add those objects to our existing structural navigation.
Created attachment 111578 [details] [review] revision 0.9 Not quite ready to call this revision 1. :-) That said. This: 1. Pylints to a 10.0 2. Passes the regression tests (we admittedly need more tests though) 3. Adds the following objects: a. Anchors b. Buttons c. Check boxes d. Combo boxes e. Entries f. Radio buttons 4. Removes redundant code. (The net code reduction seems to be around 250 lines, and that is including the addition the 6 new objects and increased comments and documentation. Around 3000 of the nearly 9000 lines were removed from Gecko's script.py.) 5. Makes it possible to add structural navigation to other scripts (hopefully without too much effort). Will, please.... What's a gentler word than "review"?... Take a gander and advise. :-) Thanks!
(Note to self: Bug 534393 is redefining chunk. The final accepted solution for that bug will need to be incorporated here.)
Joanie and I discussed this on the phone a while back, so I'm taking my name off it.
Since the new structural navigation will need to use whatever is ultimately implemented in bug 494196, marking this as a depends on that bug.
Hermann requested we implement paragraph structural navigation. Other screen readers have this functionality and adding it (assuming we only care about true paragraphs and not clumps of text separated by line breaks) will be a breeze with the new way of doing things. I think it's worth doing. Any objections?
Created attachment 112761 [details] [review] revision 2 This version: * is updated with the new chunk predicate * is updated with the new keybindings changes * uses full text variable names (e.g. snObj is now structuralNavigationObject, etc.) :-) * gets rid of the unnecessary factory (Will, you were right) * has the docs filled out * adds a useStructuralNavigationModel() to script.py * moves the 4 structural navigation related settings to settings.py (they're still in the same place in the preferences dialog) * unbinds all the keybindings I had bound for testing (e.g. combo boxes, check boxes, etc.) * Adds support for navigation by paragraph (bound to p and shift+p) * Adds support for collection+predicate matching (i.e. when we can't solely rely upon collection to determine if something is a match, but want to take advantage of the speed collection provides us). * Uses collection+predicate support both for paragraphs and for chunks/large objects (because we can find the desired object via collection, but cannot verify the character count) * Seems to work, all pylints to 10's, and passes the regression tests. * Is a HUGE change and needs lots of hammering on. Mike: I think its ready for you to test. Note that this adds a couple of files so you'll want to do autogen.sh and make. Also if you have customized your structural navigation settings, you may want to clean up or remove your Mozilla.py* files from your app-settings. Will: If you can think of anything else you asked me to do that is not mentioned in the above list, please let me know. There are still some things I want to do around structural navigation, such as look to see if we can speed up locating the previous object when near the end of a really large page, and also see if we can get an increase in performance by getting more than one match at a time. *However* given the size of this patch, I'm more interested in making sure nothing is broken and the new functionality (i.e. the new objects we've added) works. I'll open new bugs on the performance issues I've yet to address. Thanks!
With this patch in place we seem not to be repping around to the top while moving by large object. To test do the following: 1. go to www.sfgate.com/sports 2. keep pressing "o" Notice when you get to the bottom you don't circle back to the first large object.
I'll check it out. Thanks!
Created attachment 112868 [details] [review] revision 3 Thanks Mike. This solves it for me. Please test it, and the patch in general. I noticed while testing this on your sports page that, once again, using collection when going to the previous object on page with lots of objects can be painfully slow. We weren't using it with chunks prior to this patch. I'll do a little more investigation to see if I can speed it up in this patch. If not, chunk may have to go back to being a predicate-based match (either for goPrevious or period).
The reason why previous chunk via collection is so slow turns out to be an at-spi bug: When we ask for the previous match, we get all of the matches from the top of the document frame to our present location. (We're supposed to just get 1 when we ask for 1). On Mike's test case from comment 7, pressing Shift+o from the 2nd chunk (which is near the top of the page) returns 85 items. Getting those 85 items takes time. Then we have to check to see if the object is a valid chunk (right character count, etc). If it's not, we do another query and get another large number of objects we didn't ask for. Etc., Etc. I've opened bug 538680 against at-spi and bug 535023 against Orca to track it. Now to find an interim work-around so we can get this patch finalized....
Sheesh... Sorry for the spam. The tracking bug is bug 538682.
Created attachment 112881 [details] [review] revision 3a This is revision 3 with the added change that if the thing we're after is the *previous* chunk, don't use collection. That's the best I can come up with given the aforementioned collection bug. I need to re-run the regression tests, but it seems fine from functional testing, so Mike please test this one. Thanks! I also spent some time doing some performance testing: 1. When moving forward by chunk, we seem to pick up a bit of performance (not huge, but some: around 20% give or take) by using collection rather than traversing the tree ourselves. So we're using collection for goNext for chunks. 2. When moving forward by form field (be it generic form fields or specific types of them) we would pick up a bit of performance (again, not huge, but around 20%) by using collection. However, that falls apart on the bugzilla advanced search page. Those stupid 1000-item long listboxes nail us every time. :-( So form field navigation is still old-school Orca atm. I see a couple of tree traversal types in the at-spi idl that sound like they *might* be relevant to this issue (TREE_RESTRICT_CHILDREN and TREE_RESTRICT_SIBLING). But I can't find any actual docs that tell me what those do, so I'll have to experiment with them later and/or look at the code. Unless, of course, you guys already know what these things do and are willing to enlighten me. :-)
This patch seems to be working. I'd be in favor of checking it in for more community testing.
Created attachment 113087 [details] [review] one more slight tweak When proofing the patch just... one... more... time..., I noticed that I had forgotten to remove largeObjectTextLength from Gecko.script_settings. We weren't using it; we were using the new setting. But the old one needed removing. That's the only change here. This version has been pylinted and regression tested. Mike, thank you so much for hammering on this. Given the enormity of the change, I would like to postpone a commit until early/mid next week. Here's why: 1. I'd really like Will to look it over when he has the time to do so. He made a lot of great (and needed) suggestions after the initial version during a review/walk through over the phone. I believe I've made all of those changes, but I'd still appreciate another review. (Thanks in advance Will!!) 2. Murphy's law dictates that I've broken something somewhere. Given how snarky I've been lately, Karma dictates that whatever that something is, I broke it really, really well. <grin> Therefore, when I do check this in I want it to be at a time where I can drop everything and fix whatever breakage is reported immediately. This Saturday I'm teaching all day in Boston. Sunday I'll be recovering. Monday and Tuesday I'll be desperately preparing my lecture for Wednesday's class. But Wednesday through Sunday next, I'll be able to drop everything should the need arise. <smile>
Ha! - this is a *huge* patch. It looks fine to me, but there's a lot there. I trust that you've tested the bejeezus out of it, so feel free to check it in. It's definitely a good cleanup. The only I comment I have is to make sure structural_navigation.py is addede to po/POTFILES.in since it adds new strings for translation. Thanks!
Thanks Will. Committed to trunk with the additional po/POTFILES.in change. The list has been warned. Hold on tight. :-)
Hi Joannie, Might this be the right time to suggest the introduction of settings.wrappedStructureNavigation? I recall a while back people were wishing that the wrapping searches could be easely toggled. Thank you
Absolutely Jon. Please file a new RFE. Thanks!
Excellent! Thank you :)
Closing this bug as FIXED. If it turns out this uber change introduced any regressions we haven't found, new, specific bugs should be opened.