GNOME Bugzilla – Bug 490568
Implement Firefox page summary using Collections
Last modified: 2008-07-22 19:32:39 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.
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.
(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!
(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.
Created attachment 98315 [details] [review] second version of Firefox page summary using Collections
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.
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?
> 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!
Committed to trunk.
Re-opened until percent read issue is resolved.
It has been agreed that percent read can be omitted from Collection page summary. Marked as Fixed.