299622 - BiDi: Pressing End in a blank line moves the caret back, to the end of the previous line

Status: VERIFIED FIXED

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

Description

[Prognathous]

To reproduce:

1. Open the attached testcase
2. Put the caret between the two characters (in the beginning of the empty line).
3. Press End

Actual result: The caret moves back, to the end of the previous line

Expected result: The caret should always stay in the same line when End is
pressed, in this case it should remain where it is (as the line is without text)

The same problem happens in the LTR textarea, but not if the page is still
treated by Gecko as non-BiDi (another testcase is coming).

I'm marking the Severity of this bug as 'Minor', because in real life there
isn't any point in pressing End in an empty line. Nevertheless, this bug can be
useful to learn more about the otherwise buggy caret behavior in BiDi texts.

Prog.

Comment 1

[Prognathous]

Attachment: Testcase F

Comment 2

[Prognathous]

Attachment: Testcase F (non-BiDi version)

Comment 3

[Uri Bernstein (Google)]

This happens to be fixed by my patch to bug 16311. It has little to do with most
of the other BiDi caret bugs, as nsTextFrame::PeekOffset() is never entered in
this case.

Comment 4

[Uri Bernstein (Google)]

Ignore comment #3. It's no longer true after fixing the various regressions from
my original patch to bug 16311.

Comment 5

[Uri Bernstein (Google)]

Attachment: patch

I'm not sure what the original intention of this code was, but at least
following the fix to bug 16311 and the subsequent fixes to bug 305239,
PeekOffset(), in the case of HOME and END, only reverses the hint if it has a
good reason to do so. Ignoring this reversing is what led to this bug.

Comment 6

[Uri Bernstein (Google)]

Attachment: patch

As usual, forgot to remove some unrelated stuff from the patch before
submitting. Trying again.

Comment 7

[Simon Montagu :smontagu]

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

IIRC that code was intended to handle the case where the first frame on the
line was in a different direction from the paragraph direction, but in the
current code base that case also works much better with your patch.

r=me

Comment 8

[Uri Bernstein (Google)]

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

ROC - this is a bit of code removal which makes things look neater and work
better. Don't worry - I won't ask for this to go into the branch :-)

Comment 9

[Uri Bernstein (Google)]

Patch checked in to trunk by mrbkap at 2005-08-24 10:46.

Comment 10

[Uri Bernstein (Google)]

Checked in to 1.8.1 branch:
Checking in mozilla/layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.199.2.3; previous revision: 3.199.2.2
done