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 490568 - Implement Firefox page summary using Collections
Implement Firefox page summary using Collections
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: general
unspecified
Other Linux
: Normal enhancement
: 2.22.0
Assigned To: Scott Haeger
Orca Maintainers
Depends on:
Blocks: 490565 491756
 
 
Reported: 2007-10-26 16:05 UTC by Scott Haeger
Modified: 2008-07-22 19:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
first version of page summary using Collections (4.89 KB, patch)
2007-10-30 19:05 UTC, Scott Haeger
none Details | Review
second version of Firefox page summary using Collections (4.92 KB, patch)
2007-11-01 13:38 UTC, Scott Haeger
none Details | Review
third version of Firefox page summary using Collections (4.95 KB, patch)
2007-11-02 13:59 UTC, Scott Haeger
accepted-commit_now Details | Review

Description Scott Haeger 2007-10-26 16:05:18 UTC
This method should be a good proving ground to compare the efficiency of the Collection interface as compared to non-Collections techniques.  Implement it using Collections and do a performance analysis comparing the old and new methods.
Comment 1 Scott Haeger 2007-10-30 19:05:10 UTC
Created attachment 98197 [details] [review]
first version of page summary using Collections

First version of page summary using Collections.

Latest Collection patch can be found here http://bugzilla.gnome.org/show_bug.cgi?id=326516

Good test page:
/test/html/htmlpage.html

Notes:
1) This version shows about a 5-8 times improvement over the iterative version.
2) Collection version does not indicate percentage of document read.  After testing, I am not sure the iterative version is able to accurately determine the percentage of the document read if the locus of focus is on the document frame which contains lots of text.
3) This version uses the iterative path if Collections is not available.
4) Our double-click strategy is still very weak.
Comment 2 Willie Walker 2007-10-31 19:04:17 UTC
(In reply to comment #1)
> Created an attachment (id=98197) [edit]
> first version of page summary using Collections

Cool!  From an anal-retentive nit-picky stylistic point, I really don't like Hun
garian notation (http://en.wikipedia.org/wiki/Hungarian_notation).  It reminds m
e of horrific days poring through MSFT SMB/NetBIOS code.  The rest of Orca tries
 to use a fullWordCamelCase style, so Hungarian notation sticks out like a srThm
b.  :-)

So, for specifics, perhaps this would be more in line with other things done thr
oughout the code:

    collection = docFrame.queryCollection()

and...

    def _collectionPageSummary(self, collection):

In addition, I wonder if maybe the following could be simplified:

+            try:
+                docframe = self._script.getDocumentFrame()
+                icollection = docframe.queryCollection()
+            except NotImplementedError:
+                self._iterativePageSummary(obj)
+            else:
+                self._collectionPageSummary(icollection)

to something like:

+            try:
+                self._collectionPageSummary(obj)
+            except NotImplementedError:
+                self._iterativePageSummary(obj)

Where self._collectionPageSummary just re-raises or just lets the NotImplemented
Error bubble up.

> Good test page:
> /test/html/htmlpage.html

Good looking guy on that page.

> Notes:
> 1) This version shows about a 5-8 times improvement over the iterative version
.

This is good!

> 2) Collection version does not indicate percentage of document read.  After
> testing, I am not sure the iterative version is able to accurately determine
> the percentage of the document read if the locus of focus is on the document
> frame which contains lots of text.

Very interesting.  I wonder if we could provide some other information based upo
n the scrollbar (if we can find it)?  For example, something that describes the 
scrollbar itself -- how much of the total content is showing in the window and w
hat position in the content is the top line of the window.  The wording would be
 tricky, as evidenced by my lack of clarity/grammar in describing it.  :-)

> 3) This version uses the iterative path if Collections is not available.

Cool.  Thanks!

> 4) Our double-click strategy is still very weak.

I cannot remember.  What does the double-click do?

Thanks!
Comment 3 Scott Haeger 2007-11-01 13:35:38 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=98197) [edit]
> > first version of page summary using Collections
> 
> Cool!  From an anal-retentive nit-picky stylistic point, I really don't like
> Hungarian notation 

No problem.  I aim to please.

> > Good test page:
> > /test/html/htmlpage.html
> 
> Good looking guy on that page.

You must be confused with the stud on /test/html/simpleform.html ;)

> > 2) Collection version does not indicate percentage of document read.  After
> > testing, I am not sure the iterative version is able to accurately determine
> > the percentage of the document read if the locus of focus is on the document
> > frame which contains lots of text.
> 
> Very interesting.  I wonder if we could provide some other information based
> upo
> n the scrollbar (if we can find it)?  For example, something that describes the 
> scrollbar itself 

Peter and I wrestled with this idea with no luck.  I am all ears to a new strategy here.

> > 4) Our double-click strategy is still very weak.
> 
> I cannot remember.  What does the double-click do?

Page summary is hooked to the double-click of whereamI.  What ends up happening is that both a single and double click get passed on to the callback.

Comment 4 Scott Haeger 2007-11-01 13:38:30 UTC
Created attachment 98315 [details] [review]
second version of Firefox page summary using Collections
Comment 5 Willie Walker 2007-11-01 20:22:31 UTC
Looks much better!  Thanks!  What happens if this patch is used without the patch for bug 326516?  The reason I ask is that I wonder if we should hold off on committing this patch until bug 326516 is resolved.
Comment 6 Scott Haeger 2007-11-02 13:59:10 UTC
Created attachment 98375 [details] [review]
third version of Firefox page summary using Collections

This version frees the MatchRule.

I was concerned that if Collection was not supported we would get an AttributeError instead of a NotImplementedError.  This is not the case on my machine.  I am getting a NotImplementedError when a fresh trunk version of at-spi is installed.  In the case of unsupported Collection, the exception is caught and the iterative version is called.  

I sure would like to test the exception type issue on a machine that has never seen Collection.  Any takers?
Comment 7 Willie Walker 2007-11-02 20:06:41 UTC
> I sure would like to test the exception type issue on a machine that has never
> seen Collection.  Any takers?

Using the patch and trace2html on my recently installed Solaris Express Community Edition b76 machine, I can confirm that it got to the _iterativePageSummary method and it spoke what seemed to be the right stuff.  Thanks!
Comment 8 Scott Haeger 2007-11-06 16:20:49 UTC
Committed to trunk.
Comment 9 Scott Haeger 2007-11-06 17:13:00 UTC
Re-opened until percent read issue is resolved.
Comment 10 Scott Haeger 2007-11-06 21:35:47 UTC
It has been agreed that percent read can be omitted from Collection page summary.

Marked as Fixed.