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

VERIFIED FIXED in mozilla1.8.1

Status

()

defect
--
minor
VERIFIED FIXED
14 years ago
11 years ago

People

(Reporter: bugzillamozilla, Assigned: uriber)

Tracking

({fixed1.8.1})

Trunk
mozilla1.8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

14 years ago
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.
Reporter

Comment 1

14 years ago
Posted file Testcase F
Reporter

Comment 2

14 years ago
Assignee

Comment 3

14 years ago
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
Assignee

Updated

14 years ago
Status: NEW → ASSIGNED
Assignee

Comment 4

14 years ago
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
Assignee

Comment 5

14 years ago
Posted 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)
Assignee

Comment 6

14 years ago
Posted 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)
Assignee

Updated

14 years ago
Attachment #193558 - Flags: review?(smontagu)
Assignee

Updated

14 years ago
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+
Assignee

Comment 8

14 years ago
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+
Assignee

Comment 9

14 years ago
Patch checked in to trunk by mrbkap at 2005-08-24 10:46.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Assignee

Updated

14 years ago
Attachment #193559 - Flags: approval1.8.1?
Assignee

Updated

14 years ago
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+
Assignee

Comment 10

14 years ago
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

Updated

11 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.