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 563171 - src/orca/flat_review.py:getZonesFromText should clip zones based on what text is visible
src/orca/flat_review.py:getZonesFromText should clip zones based on what text...
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: general
2.24.x
Other All
: Normal normal
: 2.24.3
Assigned To: Willie Walker
Orca Maintainers
Depends on:
Blocks: 491756
 
 
Reported: 2008-12-03 22:03 UTC by Mesar Hameed
Modified: 2009-01-13 19:36 UTC
See Also:
GNOME target: ---
GNOME version: 2.23/2.24


Attachments
current code with timing statements (3.52 KB, patch)
2008-12-03 22:08 UTC, Mesar Hameed
none Details | Review
using fast code (3.52 KB, patch)
2008-12-03 22:13 UTC, Mesar Hameed
none Details | Review
clean patch (1.47 KB, patch)
2008-12-03 22:19 UTC, Mesar Hameed
none Details | Review
Pylinted patch (1.47 KB, patch)
2008-12-11 20:16 UTC, Willie Walker
needs-work Details | Review
revision 3 (1.47 KB, patch)
2009-01-07 23:34 UTC, Mesar Hameed
none Details | Review
Pylinted and tested patch (1.83 KB, patch)
2009-01-08 17:33 UTC, Willie Walker
committed Details | Review

Description Mesar Hameed 2008-12-03 22:03:10 UTC
Please describe the problem:
If we Open up a large file 5000 lines + in gedit, and we are at the end of the document, and we perform a flat-review command, it takes longer than expected before we get any feedback. This is because we walk through the text (all 5000 lines) and we work out the zones for each line.


patch to be attached.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Mesar Hameed 2008-12-03 22:08:42 UTC
Created attachment 123903 [details] [review]
current code with timing statements

this is the existing code, the new code is in place, but is not actually being used. It is simply run alongside. timing info is printed to show which bits are taking longest.
Comment 2 Mesar Hameed 2008-12-03 22:13:10 UTC
Created attachment 123904 [details] [review]
using fast code

uses the fast implementation, the patch is still printing a lot of info, just to show the diffrences, in case we wish to compare the two.
Comment 3 Mesar Hameed 2008-12-03 22:19:25 UTC
Created attachment 123905 [details] [review]
clean patch

clean patch, no timing info, no print statements, and cleaned up the code.

The code performs a binary search to locate the upper and lower area of the visible region.

Please test, and appologies, it probably needs pylinting.

thank you
Comment 4 Willie Walker 2008-12-11 16:51:26 UTC
Taking a look.  I need to debug why macaroon is giving me incorrect keycodes on OpenSolaris first, though.
Comment 5 Willie Walker 2008-12-11 20:16:50 UTC
Created attachment 124452 [details] [review]
Pylinted patch

This seems to test well with gtk-demo. I'm running the Firefox tests now, but those tend to be a little fragile and I'm seeing some failures.

Joanie, do you have a chance to check out the Firefox tests on OpenSolaris?  I'm using Orca from trunk and the Firefox that comes with OpenSolaris 2008.11: Mozilla/5.0 (X11; U; SunOS i86pc; en-US; rv:1.9.0.4) Gecko/2008111710 Firefox/3.0.4.
Comment 6 Willie Walker 2008-12-11 20:20:08 UTC
(In reply to comment #4)
> Taking a look.  I need to debug why macaroon is giving me incorrect keycodes on
> OpenSolaris first, though.

PS - when I run in a vncserver session (which is what I normally do), the tests are fine.  When I run in a regular session, the tests fail due to Up/Down/Home/End/Etc. keysyms being added to the keypad keymap.  See the "Known Issues" section of http://live.gnome.org/Orca/RegressionTesting for some xmodmap commands I just added to clear this junk out of the keymap.
Comment 7 Willie Walker 2009-01-07 22:14:53 UTC
I was able to get my test environment working again, and Joanie did a bunch of great work getting the Firefox regression tests in sync with what we're using on Orca (thanks Joanie!).

The patch causes the Firefox flat_review_text_by_line.py and flat_review_text_by_word_and_char.py tests to fail.  The main problem seems to be that the paragraphs outside the blockquotes in test/html/blockquotes.html are now being presented as 'paragraph' instead of their normal text ("On weaponry:", "On old ladies:", "On castle decor:").  Without the patch applied, things work fine.  :-(

I haven't dug into it deeper than this, but my suspicion is that the new algorithm might break when it comes to text objects consisting of only a single line.  To confirm this, I created a gedit document with a single line in it, and the patch failed for that as well (flat review spoke the text area as "blank" instead of speaking the text).

Jon - can you see if you can adjust the patch to work with text objects containing only a single line?  This patch seems like great performance improvement and I'd like to try to get it in for 2.24.3 this coming Monday.

Thanks for your work!
Comment 8 Mesar Hameed 2009-01-07 23:34:23 UTC
Created attachment 125986 [details] [review]
revision 3

appologies, a typo caused us to miss the one line case. Hopefully this should test better.
Comment 9 Willie Walker 2009-01-08 17:33:02 UTC
Created attachment 126042 [details] [review]
Pylinted and tested patch

Thanks!  This patch pylints and tests well with gtk-demo and Firefox.  I've committed it to trunk and gnome-2-24.
Comment 10 Mike Pedersen 2009-01-13 19:27:02 UTC
This seems to work well.