Note: There are a few cases of duplicates in user autocompletion which are being worked on.

finding text doesn't scroll it into view properly

RESOLVED FIXED

Status

()

Core
Layout: View Rendering
P2
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: tnikkel, Assigned: tnikkel)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(2 attachments)

(Assignee)

Description

7 years ago
Created attachment 439663 [details]
testcase

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.
(Assignee)

Comment 1

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

Updated

7 years ago
blocking2.0: --- → ?
blocking2.0: ? → final+
Priority: -- → P2

Comment 2

7 years ago
I'm seeing this on the tinderbox page, half the time the highlighted text is offscreen.
(Assignee)

Updated

7 years ago
Assignee: nobody → tnikkel
(Assignee)

Comment 3

7 years ago
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.
(Assignee)

Comment 4

7 years ago
Created attachment 488143 [details] [diff] [review]
patch

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?
Attachment #488143 - Flags: review?(roc)
Comment on attachment 488143 [details] [diff] [review]
patch

+  focusRect = focusRect + focusFrame->GetOffsetTo(anchorFrame);

+= ?
Attachment #488143 - Flags: review?(roc) → review+
(Assignee)

Comment 6

7 years ago
(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?
(Assignee)

Comment 7

7 years ago
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.
(Assignee)

Comment 9

7 years ago
http://hg.mozilla.org/mozilla-central/rev/015f5aa6ede2
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Duplicate of this bug: 553856
You need to log in before you can comment on or make changes to this bug.