Closed Bug 299842 Opened 15 years ago Closed 15 years ago
Di: Recursive caret movement in LTR lines which begin with an RTL word or character
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.
> 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.
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.
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.
This testcase shows that the problem can also occur in the reverse case (RTL textarea, strating with LTR text, using right-arrow).
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 #188429 - Attachment is obsolete: true
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
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta4
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.