In the attached testcase ctrl-f for "div", it will find the div that is off the screen to the right first, then it will select the red div but will only scroll the very right edge of it into view. Not fixed by bug 548792.

First bad revision: 5d8894904869 Robert O'Callahan — Bug 526394. Part 26: Rework nsSelection to use frames only. r=mats

I'm seeing this on the tinderbox page, half the time the highlighted text is offscreen.

nsTypedSelection::ScrollIntoView calls nsTypedSelection::GetSelectionAnchorGeometry (which used to be called nsTypedSelection::GetSelectionRegionRectAndScrollableView) to get a rect relative to a frame to scroll into view. GetSelectionAnchorGeometry returns a zero width rect at the offset of the focus point in the frame containing the focus point. In this case the focus point is the end of the selection, so we just scroll into view the end of the selection. GetSelectionAnchorGeometry used to have some hacks in it for this situation: it used to extend the rect by 1/4 the width of the nearest scrollable view if the rect was not scrolled into view. Seems like the correct thing to do here is to include the whole selection in the rect we want to scroll into view.

This patch adds a new type of selection region to scroll into view: the whole selection. We just compute a rect that encompasses the anchor and focus rect and scroll that into view. Does this need an IID bump and/or to get in for beta 7?

Comment on attachment 488143 [details] [diff] [review] patch + focusRect = focusRect + focusFrame->GetOffsetTo(anchorFrame); += ?

(In reply to comment #5) > + focusRect = focusRect + focusFrame->GetOffsetTo(anchorFrame); > > += ? Changed to +=. Does the nsISelectionController.idl change need an IID bump and/or to get in before beta 7?

And to remind myself, I need to write a test for this.

(In reply to comment #4) > Does this need an IID bump and/or to get in for beta 7? I don't think it needs an IID bump.