Closed Bug 301033 Opened 15 years ago Closed 15 years ago

Bidi: Caret placed in wrong position when arrowing over a single reverse-direction character

Categories

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

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.8beta4

People

(Reporter: uriber, Assigned: uriber)

References

Details

Attachments

(2 files)

In a textarea or text field, when a single LTR character is surrounded by RTL
text, or a single RTL character is surrounded by LTR text, using an arrow key to
move over the character places the caret at an incorrect position. Using that
arrow key once again places the caret at the expected position (i.e. the net
effect of the two keypresses is correct).

This bug, as described, is currently not reproducable in the case of a single
LTR character in RTL text inside an RTL textbox, due to bug 299239. 

A detailed testcase (and a patch) coming up.

Bug history:

This bug was originally filed as bug 205679, then described in bug 207186,
comment 17 (and closed as a duplicate of that bug). The bug was not reproducable
between 2004-08-04 and 2005-07-14 due to a regression caused by the original
patch to bug 254278 (and fixed by the second patch on that bug).
I chose to file a new bug rather than re-opening 205679 because the original
description on that bug is somewhat inaccurate.
Attached file testcase
1. On the first (LTR) textarea, in the first line, place the caret between the
"c" and the Hebrew chracter, and press the right arrow. The caret doesn't move.
Pressing the right arrow agian moves the caret to between the "d" and the "e"
(as expected after two arrow presses). The same thing happens when placing the
carety between the Hebrew character and the "d", and using the left arrow.

2. Still on the first textarea, in the second line, placing the caret
immediately to the left of the "a" and pressing the right arrow moves the caret
to the extreme right of the line. placing the caret to the right of the "a" and
pressing the left arrow moves the caret to the extreme left of the line. A
second use of the same arrow key does not behave here as described in the
original summary, likely due to bug 299239.

3. On the second (RTL) area, on the first line, the behaviour is similar to
what I described in (3) - the caret moves to the end of the line. However, here
an additional keypress on the same arrow key moves the caret to the expected
position (one letter away from the Hebrew character).

4. On the second line of the RTL textarea this bug is currently not
reproducable - the caret simply won't move when reaching the "a", due to bug
299239.
Attached patch patchSplinter Review
The problem was that when MoveCaret() calls BidiLevelFromMove(), mHint still
contains the previous hint: the new hint is stored in tHint, in case the
original mHint will have to be restored. (this was done as part of the fix for
bug 82151, from which this bug is probably a regression - but this is too old
for me to verify).
The solution is to pass the up-to-date hint (tHint) as an argument to
BidiLevelFromMove(), and use it there instead of using mHint.
Attachment #189546 - Flags: review?(smontagu)
Status: NEW → ASSIGNED
*** Bug 205679 has been marked as a duplicate of this bug. ***
Comment on attachment 189546 [details] [diff] [review]
patch

Simon is on vacation until 17/8, and this is a relatively simple fix which I
hope to get into 1.8 - so moving request review to roc.
Attachment #189546 - Flags: review?(smontagu) → review?(roc)
Attachment #189546 - Flags: superreview?(roc)
Attachment #189546 - Flags: superreview?(roc)
Attachment #189546 - Flags: superreview+
Attachment #189546 - Flags: review?(roc)
Attachment #189546 - Flags: review+
Attachment #189546 - Flags: approval1.8b4?
Attachment #189546 - Flags: approval1.8b4? → approval1.8b4+
Checking in nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.199; previous revision: 3.198
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta4
Status: RESOLVED → VERIFIED
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.