Closed Bug 299622 Opened 15 years ago Closed 15 years ago

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

Categories

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

defect
Not set
minor

Tracking

()

VERIFIED FIXED
mozilla1.8.1

People

(Reporter: bugzillamozilla, Assigned: uriber)

References

Details

(Keywords: fixed1.8.1)

Attachments

(3 files, 1 obsolete file)

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.
Attached file Testcase F
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.
Depends on: 16311
Status: NEW → ASSIGNED
Ignore comment #3. It's no longer true after fixing the various regressions from
my original patch to bug 16311.
No longer depends on: 16311
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #193558 - Flags: review?(smontagu)
Attached patch patchSplinter Review
As usual, forgot to remove some unrelated stuff from the patch before
submitting. Trying again.
Attachment #193558 - Attachment is obsolete: true
Attachment #193559 - Flags: review?(smontagu)
Attachment #193558 - Flags: review?(smontagu)
Hardware: PC → All
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
Attachment #193559 - Flags: review?(smontagu) → review+
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 :-)
Attachment #193559 - Flags: superreview?(roc)
Attachment #193559 - Flags: superreview?(roc) → superreview+
Patch checked in to trunk by mrbkap at 2005-08-24 10:46.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Attachment #193559 - Flags: approval1.8.1?
Attachment #193559 - Flags: approval1.8.1? → branch-1.8.1?(roc)
Attachment #193559 - Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1+
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
Keywords: fixed1.8.1
Target Milestone: mozilla1.9alpha → mozilla1.8.1
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.