330732 - Caret not correctly scrolled into view in RTL textarea with a horizontal scrollbar

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Uri Bernstein]

When an RTL textarea has long unbreakable text in it, it now correctly shows a horizontal scrollbar which allws scrolling to the left.
However, when the caret is moved left using the left-arrow key, the textarea doesn't horizontally scroll to bring the caret into view.
Furthermore, when the line is long enough, using the arrow keys near the left edge of the text causes the textarea to scroll towards the middle, bringing the caret out of view.

I first mentioned this issue in bug 192767 comment #70.

Testcase coming up.

Comment 1

[Uri Bernstein]

Attachment: testcase

Place the caret on the first line and use left-arrow to move it beyond the left edge of the textarea. notice that the caret moves out of view, and the textarea doesn't scroll.

Now scroll all the way to the left, place the caret near the left edge of the text, and press the arrow keys twice. Notice that the scrollbar scrolls towards the middle, leaving the caret out of view.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (diff -u)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (diff -ub)

The relevant changes here are in nsSelection.cpp; the nsLayoutUtils change is just something I noticed that didn't handle right-side scrollbars, and the nsGfxScrollFrame change is a comment for something that I need to remind myself of (and which may be interesting to deal with during view removal).

There were two bits of code in nsSelection.cpp that were clamping to the scrollable area incorrectly.  I removed both, since they're unneeded for clamping, since nsScrollPortView does that fine.  But one of them (getting the rect for a selection region) was important in the case where there were two nested scrollable areas and the inner one didn't allow the entire region to be scrolled to; it fixed erroneous scrolling of the outer.  I've extended that fix so that it works for more than the innermost two scrollable areas, but I haven't applied it to the point scrolling routines.

That said, I should probably do some testing of such scrolling cases; I haven't done so yet.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, for what the ScrollPointIntoView stuff is used for, we don't want to do that clipping; it seems to be used only for auto-scrolling as a user drags, and we probably want to keep going in that case when they drag out of the innermost scrolling view.  So I'll remove those XXXldb comments and move the function closer to where it's used, I think.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

So should I expect a new patch or do you want me to review this one?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (diff -u)

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (diff -ub)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk.

Comment 9

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).