Closed Bug 330461 Opened 14 years ago Closed 13 years ago

Bidi: In a line ending with reverse-direction text, "End" does not move the caret to the visual end of the line, if the next line starts with reverse-direction text

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: uriber, Assigned: uriber)

References

Details

Attachments

(2 files, 2 obsolete files)

In an LTR textarea, if a line ends with RTL text, and the next line begins with RTL text, then pressing "End" on the first line does not cause the caret to be displayed at the visual end (left) of the line. Instead the caret is placed before the RTL text.

The same is true when switching "LTR" and "RTL" in the description above.

This is not a regression.

Testcase coming up.
Attached file testcase
Instructions included.
This will likely be fixed by fixing bug 263359, because that would make the BRFrame at the logical end of the line also be at its visual end.
Depends on: 263359
Bug 263359 will fix the text area case, but what about caret browsing in HTML with equivalent content?
(In reply to comment #3)
> Bug 263359 will fix the text area case, but what about caret browsing in HTML
> with equivalent content?
> 

Yes, good point.
Attached patch patch (obsolete) — Splinter Review
For non-text frames (e.g., the BRFrame in this case), for which frameStart == frameEnd == 0, use the hint to determine whether to look at the previous or next frame (instead of arbitrarily looking at the previous frame).
Assignee: mozilla → uriber
Status: NEW → ASSIGNED
Attachment #218352 - Flags: review?(smontagu)
Attached patch patch (obsolete) — Splinter Review
The correct file, this time.
Attachment #218352 - Attachment is obsolete: true
Attachment #218353 - Flags: review?(smontagu)
Attachment #218352 - Flags: review?(smontagu)
Attachment #218353 - Flags: review?(smontagu) → review+
Attachment #218353 - Flags: superreview?(roc)
Comment on attachment 218353 [details] [diff] [review]
patch

Actually, upon further consideration, I don't think this patch is correct. It just happened to fix the problem by chance.

Simon, apologies for wasting your time on it.
Attachment #218353 - Attachment is obsolete: true
Attachment #218353 - Flags: superreview?(roc)
Assignee: uriber → nobody
Status: ASSIGNED → NEW
QA Contact: zach → layout.bidi
What I said in comment 2 about bug 263359 is probably true for bug 339786, assuming the BR frame will be treated as whitespace (which it probably should be).

Alternatively, I can hack this in nsFrameSelection::GetPrevNextBidiLevels, by setting frameAfter to null if it would have otherwise been a BR frame. That patch superficially conflicts with my patch for bug 333883, so I'll wait for that one to go in before posting it.
Depends on: 333883, 339786
No longer depends on: 263359
Attached patch patch v2Splinter Review
As described in the previous comment: just pretend the brframes don't exist (i.e., we're really at the edge of the line when we're before a breframe).
Assignee: nobody → uriber
Status: NEW → ASSIGNED
Attachment #232681 - Flags: review?(smontagu)
Attachment #232681 - Flags: review?(smontagu) → review+
Attachment #232681 - Flags: superreview?(roc)
Attachment #232681 - Flags: superreview?(roc) → superreview+
Checked in:
Checking in layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.259; previous revision: 3.258
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: layout.bidi → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.