Bidi: Caret doesn't move correctly when arrowing where line wraps, if the wrapped line starts with a single reverse-direction character

RESOLVED FIXED in mozilla1.8.1

Status

()

--
minor
RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: uriber, Assigned: uriber)

Tracking

({fixed1.8.1, testcase})

Trunk
mozilla1.8.1
fixed1.8.1, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

13 years ago
In a LTR textarea, when a line wraps, and following the point where it wraps is
a single RTL character, pressing right-arrow at the beginning of the wrapped
line (next to the RTL character) does not move the caret. Pressing right arrow
again moves the caret two characters to the right. 

The same is correct for the reverse situation (RTL textarea, LTR character,
using right arrow).

The behavior somewhat changes if the patch for bug 305798 is applied: The first
arrow does not move the caret (as before), but the next arrow moves the caret
one character, as expected.

This happens consistently since the fix for bug 303399. Before that, it only
happened sporadically, depending on previous selection actions done outside the
textarea.

Testcase coming up.
(Assignee)

Comment 1

13 years ago
Created attachment 199724 [details]
testcase

Self documenting.

NOTE: depending on your default fonts/sizes, you might need to adjust the
number of characters in the first line of each textarea so that the
reverse-direction character will appear at the beginning of the second line.
(Assignee)

Updated

13 years ago
Keywords: testcase
(Assignee)

Comment 2

13 years ago
Created attachment 199762 [details] [diff] [review]
patch v1

This bug turns out to be a leftover from bug 301033, and this patch completes
the fix to that bug.
In 301033 I changed BidiLevelFromMove() to receive a aHint parameter, and use
it instead of mHint (because MoveCaret() only sets mHint correctly before it
exists,  much after it calls BidiLevelFromMove()).
However, BidiLevelFromMove() is still calling GetPrevNextBidiLevels(), which
uses mHint. So we need to pass the hint as a parameter to
GetPrevNextBidiLevels() as well.

Instead of changing all the callers to GetPrevNextBidiLevels() to add the extra
parameter, I created a private version of it which receives the parameter, and
wrapped it with a method with the original signature which passes mHint, so the
only caller which had to be changed was the one I care about.
Attachment #199762 - Flags: review?(roc)
Attachment #199762 - Flags: superreview+
Attachment #199762 - Flags: review?(roc)
Attachment #199762 - Flags: review+
Checking in layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.206; previous revision: 3.205
done
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Updated

13 years ago
Attachment #199762 - Flags: approval1.8.1?
(Assignee)

Updated

13 years ago
Attachment #199762 - Flags: approval1.8.1? → branch-1.8.1?(roc)
Attachment #199762 - Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1+
(Assignee)

Comment 4

13 years ago
Checked in to 1.8 branch:
Checking in mozilla/layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.199.2.6; previous revision: 3.199.2.5
done
Keywords: fixed1.8.1
Target Milestone: mozilla1.9alpha → mozilla1.8.1

Updated

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