Closed
Bug 288789
Opened 19 years ago
Closed 19 years ago
BiDi - In an RTL textbox, caret moves to wrong position when using arrow keys to move into a line containing LTR text
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
VERIFIED
FIXED
mozilla1.8beta4
People
(Reporter: uriber, Assigned: uriber)
References
(Blocks 1 open bug)
Details
(Keywords: regression, rtl)
Attachments
(3 files, 1 obsolete file)
807 bytes,
text/html
|
Details | |
6.90 KB,
patch
|
smontagu
:
review+
roc
:
superreview+
benjamin
:
approval1.8b4+
|
Details | Diff | Splinter Review |
2.75 KB,
patch
|
smontagu
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
In an RTL textbox, if a line contains LTR characters embedded in RTL text, using the arrow keys to move into this line (from the previous line or next line), the caret appears at the beginning/end of the RTL fragment, rather than at the beginning/end of the line. Please notice that this is a regression. Unlike the various problems described in bug 207186, this bug was not on the Aviary branch - it only appears on the trunk.
Assignee | ||
Comment 1•19 years ago
|
||
Self-explaining testcase
Assignee | ||
Comment 2•19 years ago
|
||
Bah - I got the directions messed up a bit. The summary should read: "... the caret appears at the beginning/end of the *LTR* fragment, rather than at the beginning/end of the line."
Summary: BiDi - In an RTL textbox, caret moves to wrong position when using arrow keys to move into a line containing RTL text → BiDi - In an RTL textbox, caret moves to wrong position when using arrow keys to move into a line containing LTR text
Assignee | ||
Comment 3•19 years ago
|
||
This regressed between 2004-11-25 and 2004-12-05. Unfortunately, I can't find in the archives any builds in between these two dates (although I know such builds were made), so I can't give a more precise date. It should be noted that the Aviary landing took place approximately at that time frame.
Comment 4•19 years ago
|
||
(In reply to comment #3) > This regressed between 2004-11-25 and 2004-12-05. > Unfortunately, I can't find in the archives any builds in between these two > dates (although I know such builds were made), so I can't give a more precise date. > > It should be noted that the Aviary landing took place approximately at that time > frame. Bug 256833 and Bug 256835 are possible candidates.
Assignee | ||
Comment 5•19 years ago
|
||
Indeed, backing out the patch to bug 256835 fixes this one. I'll add a note on that bug.
I'll work on it when I've time.
Assignee: mkaply → ginn.chen
OS: MacOS X → All
Hardware: Macintosh → All
Assignee | ||
Comment 7•19 years ago
|
||
Requesting 1.8b3 block. BiDi caret movement is buggy enough even without this regression (see bug 265070). Having the situation worsen is not something people are looking forward to.
Blocks: BidiCaret
Flags: blocking1.8b3?
Updated•19 years ago
|
Flags: blocking1.8b3? → blocking1.8b3-
Assignee | ||
Comment 8•19 years ago
|
||
Taking. It seems like Ginn's patch only exposed a deeper bug, which I'm trying to fix.
Assignee: ginn.chen → uriber
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•19 years ago
|
||
Make nsFrameList::GetPrevVisualFor() and nsFrameList::GetNextVisualFor() actually return the previous (and next) visual frame, based on the paragraph direction. (before the patch, "next" meant "right" and "prev" meant "left"). This also makes the logic in nsFrame::GetFrameFromDirection() somewhat simpler, as the "lineJump" case is no longer a special case.
Attachment #187360 -
Flags: review?(smontagu)
Assignee | ||
Comment 10•19 years ago
|
||
Requesting blocking1.8b4. This is a very bad regression, and I have a proposed fix.
Flags: blocking1.8b4?
Comment 11•19 years ago
|
||
Comment on attachment 187360 [details] [diff] [review] patch The logic seems correct, but I can't test this right now, so please test with as many combinations as possible of text in different directions, with line breaks between words in the opposite direction from the paragraph direction, and with line breaks between runs in opposite directions from each other.
Attachment #187360 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 12•19 years ago
|
||
Comment on attachment 187360 [details] [diff] [review] patch I tested this in the various cases outlined by Simon, and it seems fine in all of them (there are still glitches with the visual placement of the caret in certain situations, but those are independent of this bug).
Attachment #187360 -
Flags: superreview?(roc)
Attachment #187360 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Updated•19 years ago
|
Attachment #187360 -
Flags: approval1.8b4?
Updated•19 years ago
|
Attachment #187360 -
Flags: approval1.8b4? → approval1.8b4+
Assignee | ||
Comment 13•19 years ago
|
||
I'll ask someone to check this in sometime next week, when I'll be available to deal with possible regressions.
Updated•19 years ago
|
Flags: blocking1.8b4? → blocking1.8b4+
Assignee | ||
Comment 14•19 years ago
|
||
Patch checked in by timeless at 2005-08-02 10:19.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•19 years ago
|
Target Milestone: --- → mozilla1.8beta4
Updated•19 years ago
|
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 15•17 years ago
|
||
Simple mochitest for this.
Attachment #290025 -
Flags: review?(smontagu)
Comment 16•17 years ago
|
||
Comment on attachment 290025 [details] [diff] [review] my first mochitest Nice!
Attachment #290025 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 17•17 years ago
|
||
Now in the right directory, and including Makefile.in.
Attachment #290025 -
Attachment is obsolete: true
Attachment #290129 -
Flags: review?(smontagu)
Updated•17 years ago
|
Attachment #290129 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 18•17 years ago
|
||
Comment on attachment 290129 [details] [diff] [review] mochitest, take 2 Hi roc, what are the review requirements for layout tests? do I need an sr+? Do I even need an r+, officially?
Attachment #290129 -
Flags: superreview?(roc)
Comment on attachment 290129 [details] [diff] [review] mochitest, take 2 review for tests is optional. If you feel comfortable with the test, you can just land it.
Attachment #290129 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Updated•16 years ago
|
Flags: in-testsuite+
Comment 20•16 years ago
|
||
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl
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.
Description
•