BiDi: Recursive caret movement in LTR lines which begin with an RTL word or character

VERIFIED FIXED in mozilla1.8beta4

Status

()

VERIFIED FIXED
14 years ago
10 years ago

People

(Reporter: bugzillamozilla, Assigned: uriber)

Tracking

({rtl})

Trunk
mozilla1.8beta4
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

14 years ago
To reproduce:

1. Open https://bugzilla.mozilla.org/attachment.cgi?id=137983
2. In the second textarea (LTR), put the caret anywhere on the Hebrew word 
   ("עברית").‎
3. Hold the LeftArrow button pressed.

Actual result:
The caret moves recursively to left. Every time it reaches the beginning of the
line (leftmost part), it comes back in the beginning of the Hebrew word (to
right of the word).

Expected result:
The caret should stop at the beginning of the line. This is the expected
behavior in Visual Caret Movement, as is the convention in Gecko.

Tested with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050705 Firefox/1.0+

First reported in Bug 207186, comment #19

Uri, if I understand correctly, your patch for Bug 299239 should also fix this
one. If so, please close this as WORKSFORME after you get that patch checked in.

Thanks,

Prog.
(Assignee)

Comment 1

14 years ago
> Uri, if I understand correctly, your patch for Bug 299239 should also fix this
> one. If so, please close this as WORKSFORME after you get that patch checked in.

No, this is a separate issue, for which I attached a patch to bug 207186. That
patch was checked in but caused a regression, so it was backed out. I then
suggested an alternative patch. I'll post both these patches here shortly.
(Assignee)

Comment 2

14 years ago
Created attachment 188429 [details] [diff] [review]
patch A

This is the original patch I proposed in bug 207186 comment 37. It got
r+:smontagu and sr+:roc, and was checked in, but it caused a regression: bug
299371, so it was backed out.
I'm still not sure if the approach here (PeekOffset() returning an error code
when there is nowhere to go) was really wrong, or whether it merely exposed a
problem elsewhere in the code.
(Assignee)

Comment 3

14 years ago
Created attachment 188432 [details] [diff] [review]
patch B

An alternative patch, originally posted on bug 207186, comment #50.
The idea here is to make sure that nsFrame::GetFrameFromDirection does not flip

mPreferLeft unless it succeeds.

This does not cause the regression caused by patch A.
Attachment #188432 - Flags: review?(smontagu)
(Assignee)

Comment 4

14 years ago
Created attachment 188435 [details]
testcase

This testcase shows that the problem can also occur in the reverse case (RTL
textarea, strating with LTR text, using right-arrow).
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 5

14 years ago
Comment on attachment 188432 [details] [diff] [review]
patch B

Since I got no response on bug 299371, let's just go with option "B", which
fixes this bug without causing a regression.
Since Simon is on vacation, and this is a small, simple, patch - moving review
request to roc.
Attachment #188432 - Flags: superreview?(roc)
Attachment #188432 - Flags: review?(smontagu)
Attachment #188432 - Flags: review?(roc)
(Assignee)

Updated

14 years ago
Attachment #188429 - Attachment is obsolete: true
Attachment #188432 - Flags: superreview?(roc)
Attachment #188432 - Flags: superreview+
Attachment #188432 - Flags: review?(roc)
Attachment #188432 - Flags: review+
(Assignee)

Updated

14 years ago
Attachment #188432 - Flags: approval1.8b4?

Updated

14 years ago
Attachment #188432 - Flags: approval1.8b4? → approval1.8b4+
Checking in nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.573; previous revision: 3.572
done
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta4
(Reporter)

Updated

14 years ago
Status: RESOLVED → VERIFIED

Comment 7

11 years ago
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl

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.