GNOME Bugzilla – Bug 534393
Moving by large object in firefox can skip text
Last modified: 2008-06-03 20:10:02 UTC
1. Open firefox and navigate to http://www.channelinsider.com/c/a/Reviews/Lenovo-Launches-Linux-Laptop-and-Leaves-Lots-of-Questions/ 2. attempt to read this article by pressing "o" to move through the article by large object. Notice that when you get to the list of bulletted items that moving by large object skips right over them. I wonder if we are skipping over HTML lists and perhaps we shouldn't.
Created attachment 111634 [details] [review] revision 1 Adds lists into the mix as Mike suggested. Note that this patch does base its character count on list item text that is *not* an embedded object character (i.e. we don't want this to start landing on a list of links -- the very thing many users are trying to skip over with this feature). Pylinted, but not yet regression tested. I plan to try to squash a few bugs tonight and run the tests all at once as the FF tests take 1.25 hours give or take.... Please test.
Created attachment 111643 [details] [review] revision 2: Don't be so quick to point the "useless" finger Mike pointed out via IRC that revision 1 was still skipping over text he thought it should not be skipping over. The text in question was the beginning of the large section which made up the article. That section had children which were deemed useful; however the start of the section was deemed useless because of all the embedded object characters in it. So this version takes a different approach: Rather than dismissing things because they fail the isUselessObject test, see how much text they have. If the number of characters is greater than our minimum character count for chunkhood, see what percentage of the total number of characters are EOCs. If there are a lot of EOCs, *then* dismiss it. I picked 15% as the golden number based on testing. If we go lower than 12%, we'll miss the chunk of text Mike specifically pointed out to me as something we should not miss. Mike, please take this for a spin not just on the site in the bug, but also on the various and sundry other sites you use large object navigation on and let me know if we're picking up too many "non-chunks". Specific examples of false positives will be appreciated. Thanks!
Created attachment 111690 [details] [review] revision 3 Okay, this is bug is probably going to start triggering those nightmares I had a year or so ago where form text chases me incessantly demanding, "Am I a label, am I a label?!?" <grin> Yes, our matchChunk predicate is taking a decided turn toward the heuristic.... Mike gave me a personal document "off bugzilla" which illustrated some false positives. Thanks Mike! Based on that, this version handles tables a bit differently, still adds lists into the mix, and introduces two guesses for everything else. Guess 1 is a little more liberal than my gut is telling me to make it (I'm tempted to find a factor by which to multiply the largeObjectTextLength). But I'd rather let Mike test this first and show me more false positives than go to conservative and weed out valid chunks inadvertently. So please have at it and let me know.
Created attachment 111691 [details] [review] pylinted version Please test. Thanks!
This patch seems to be working nicely. I think it's good to check in.
Woohoo! :-) Thanks Mike. Patch committed to trunk. Moving to pending.