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

RESOLVED FIXED

Status

()

RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: uriber, Assigned: dbaron)

Tracking

({rtl})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

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

Comment 1

13 years ago
Created attachment 215319 [details]
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.
Created attachment 215336 [details] [diff] [review]
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.
Attachment #215336 - Flags: superreview?(roc)
Attachment #215336 - Flags: review?(roc)
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.
So should I expect a new patch or do you want me to review this one?
Created attachment 215855 [details] [diff] [review]
patch (diff -ub)
Attachment #215336 - Attachment is obsolete: true
Attachment #215855 - Flags: superreview?(roc)
Attachment #215855 - Flags: review?(roc)
Attachment #215336 - Flags: superreview?(roc)
Attachment #215336 - Flags: review?(roc)
Attachment #215855 - Flags: superreview?(roc)
Attachment #215855 - Flags: superreview+
Attachment #215855 - Flags: review?(roc)
Attachment #215855 - Flags: review+
Fix checked in to trunk.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 9

11 years ago
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl

Updated

10 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.