330461 - Bidi: In a line ending with reverse-direction text, "End" does not move the caret to the visual end of the line, if the next line starts with reverse-direction text

Status: RESOLVED FIXED

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

Description

[Uri Bernstein]

In an LTR textarea, if a line ends with RTL text, and the next line begins with RTL text, then pressing "End" on the first line does not cause the caret to be displayed at the visual end (left) of the line. Instead the caret is placed before the RTL text.

The same is true when switching "LTR" and "RTL" in the description above.

This is not a regression.

Testcase coming up.

Comment 1

[Uri Bernstein]

Attachment: testcase

Instructions included.

Comment 2

[Uri Bernstein]

This will likely be fixed by fixing bug 263359, because that would make the BRFrame at the logical end of the line also be at its visual end.

Comment 3

[Simon Montagu :smontagu]

Bug 263359 will fix the text area case, but what about caret browsing in HTML with equivalent content?

Comment 4

[Uri Bernstein]

(In reply to comment #3)
> Bug 263359 will fix the text area case, but what about caret browsing in HTML
> with equivalent content?
> 

Yes, good point.

Comment 5

[Uri Bernstein]

Attachment: patch

For non-text frames (e.g., the BRFrame in this case), for which frameStart == frameEnd == 0, use the hint to determine whether to look at the previous or next frame (instead of arbitrarily looking at the previous frame).

Comment 6

[Uri Bernstein]

Attachment: patch

The correct file, this time.

Comment 7

[Uri Bernstein]

Comment on attachment 218353 [details] [diff] [review]
patch

Actually, upon further consideration, I don't think this patch is correct. It just happened to fix the problem by chance.

Simon, apologies for wasting your time on it.

Comment 8

[Uri Bernstein]

What I said in comment 2 about bug 263359 is probably true for bug 339786, assuming the BR frame will be treated as whitespace (which it probably should be).

Alternatively, I can hack this in nsFrameSelection::GetPrevNextBidiLevels, by setting frameAfter to null if it would have otherwise been a BR frame. That patch superficially conflicts with my patch for bug 333883, so I'll wait for that one to go in before posting it.

Comment 9

[Uri Bernstein]

Attachment: patch v2

As described in the previous comment: just pretend the brframes don't exist (i.e., we're really at the edge of the line when we're before a breframe).

Comment 10

[Uri Bernstein]

Checked in:
Checking in layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.259; previous revision: 3.258
done