GNOME Bugzilla – Bug 433596
Firefox fails to find next item, scroll
Last modified: 2007-05-11 19:56:08 UTC
When the next item is just outside the document frame viewport, LSR will consider it invisible and announce that it has reached the end of the visible view. This logic is correct for other applications, but not for Web pages where we want to scroll the next item into view. We cannot simply disable testing for visibility, because there may be items in the viewport that are truly invisible and marked as such by Firefox. We cannot override this logic per se as it is part of the item walker. The first solution that comes to mind is to try to move the caret to the next item, visible or not, then do the actual navigation. Perhaps this could tie into our peek ahead logic in the FirefoxReview task?
Created attachment 87095 [details] [review] patch for Firefox fails to find next item, scroll Tested with next/prev item, word, and char.
Comment on attachment 87095 [details] [review] patch for Firefox fails to find next item, scroll >+ # search for all nodes when peeking, otherwise use user setting >+ if self.peek: >+ onlyvisible = False >+ else: >+ onlyvisible = self.perk_state.OnlyVisible Hmm. Does (will) peeking always imply only visible is False? It probably does for Firefox, but perhaps we should decouple the two with another param for future scripts. >+ # moving caret programmatically scrolls screen and brings por into view. >+ # need to set the next item to account for lists >+ if self.getAnchorTaskId().find(' next ') > 0: >+ ni = self.getNextItem(por=next_por, wrap=True) >+ else: >+ ni = self.getPrevItem(por=next_por, wrap=True) >+ self.setAccCaret(por=ni) >+ Do we want this code to move from down there to up here? Guess I'm looking at it out of context in the patch. But does the meaning change now that we're doing it in a different place in the giant if statement? > # now determine whether we're 1) in doc to in doc, 2) out of doc to in doc, > # 3) in doc to out of doc, or 4) out of doc to out of doc >- > # navigating within document or into the document (1 and 2) > if next_in_doc: > # check if the item should get the focus >@@ -947,8 +954,6 @@ > # execute the anchor, but do not let its immediate chains run too since > # that would end up re-executing this task ad infinitum > self.doTask(self.getAnchorTaskId(), chain=False) >- # moving caret programmatically scrolls screen and brings por into view. >- self.setAccCaret() > return propagate > > class ReadReviewRole(Task.Task):
Created attachment 87152 [details] [review] next patch for Firefox fails to find next item, scroll Addition keyword argument 'onlyvisible=None' added to ReviewPerk:NextItem/PrevItem. This overrides the user setting 'OnlyVisible'. From the previous comment: # moving caret programmatically scrolls screen and brings por into view. >+ # need to set the next item to account for lists >+ if self.getAnchorTaskId().find(' next ') > 0: >+ ni = self.getNextItem(por=next_por, wrap=True) >+ else: >+ ni = self.getPrevItem(por=next_por, wrap=True) >+ self.setAccCaret(por=ni) > Do we want this code to move from down there to up here? Guess I'm looking at > it out of context in the patch. But does the meaning change now that we're > doing it in a different place in the giant if statement? Yes, the meaning does change. It seems that it needs to be called before any set focus/caret calls. I moved it to a much better spot within the first if (navigating within doc frame)
+ # moving caret programmatically scrolls screen and brings por into view. + # need to set the next item to account for lists. Must be called before + # any other set focus type calls + if self.getAnchorTaskId().find(' next ') > 0: + ni = self.getNextItem(por=next_por, wrap=True) + else: + ni = self.getPrevItem(por=next_por, wrap=True) + self.setAccCaret(por=ni) Can you explain this part of the fix to me? I'm not sure why you need to look at the next item after you've already peeked ahead at the next POR. It's like you're looking at the next, next item.
Yes, I did have to look at the next, next item which setAccCaret() on the item after the bullet. Otherwise, the screen did not scroll. I believe the problem is due to Firefox not responding to the setAccCaret on the bullet. This creates a situation where the bullet is still invisible so the walker moves out of the document to the lower status/find bar.
OK. That makes sense. We're basically going a bit ahead of the next item to improve chances that the next item is properly exposed. Go ahead and commit this patch.
committed to repository.