Closed Bug 330460 Opened 15 years ago Closed 15 years ago

Bidi: After moving back to end of line ending with reverse-direction text, "backspace" misbehaves


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

Not set





(Reporter: uriber, Assigned: uriber)




(3 files, 3 obsolete files)

In an LTR textarea, when a line ends with RTL text, using left-arrow to move into the end of the line, and then pressing backspace, results in the last letter of the RTL text being seleted, and the caret remaining at the end (right) of the line.

Expected: no deletion should actually happen, the caret should move to the left of the RTL text.

The symmetric case (RTL textarea) also has the same problem, but only since the fix to bug 303884 has landed. In other words, fixing bug 303884 caused the two cases to be symmetrical (and broken).

Annotated testcase coming up.
Attached file testcase
Instructions included.
Attached file testcase #2
This is an HTML testcase (for Composer), showing a similar problem with an inline container instead of a line break.
These problems are the result of the "more fundamental problems" mentioned by Simon in bug 303781 comment #5.
Attached patch patch (obsolete) — Splinter Review
So, after trying to think what CheckBidiLevelForDeletion() should be doing, I realized that we already have a method which does exactly that (I thought) - namely, nsSelection::GetPrevNextBidiLevels(). I had to jump trough some hoops in order to actually use it, but once I did it, this bug (as well as bug 330457) were fixed.
But then I discovered that nsSelection::GetPrevNextBidiLevels() doesn't look beyond line boundaries, which caused the LTR testcase for bug 303781 to break.
Removing the line boundary restriction caused havoc with caret positioning, so I had to add a parameter (aJumpLines) to GetPrevNextBidiLevels to specify which of the behaviors I want (it's false everywhere, except for the new callee).
Assignee: mozilla → uriber
Attachment #215176 - Flags: review?(smontagu)
Attached patch same patch, diff -w (obsolete) — Splinter Review
Since most of the change in GetPrevNextBidiLevels() is indentation, here's apatch created with diff -w.
Blocks: 330457
Attached patch patch (obsolete) — Splinter Review
oops, forgot to include nsTextEditRules.h.
Attachment #215176 - Attachment is obsolete: true
Attachment #215247 - Flags: review?(smontagu)
Attachment #215176 - Flags: review?(smontagu)
Take care of beginning/end of document (or, actually, beginning/end of scrollframe) cases.
Attachment #215177 - Attachment is obsolete: true
Attachment #215247 - Attachment is obsolete: true
Attachment #215755 - Flags: review?(smontagu)
Attachment #215247 - Flags: review?(smontagu)
Attachment #215755 - Flags: review?(smontagu) → review+
Attachment #215755 - Flags: superreview?(roc)
Comment on attachment 215755 [details] [diff] [review]
patch v2 (diff -w)

+  NS_IMETHOD GetPrevNextBidiLevels(nsPresContext *aPresContext, nsIContent *aNode, PRUint32 aContentOffset, PRBool aJumpLines,

Please document the new parameter.
Attachment #215755 - Flags: superreview?(roc) → superreview+
Checked in, with new parameter documented:
Checking in editor/libeditor/text/nsTextEditRulesBidi.cpp;
/cvsroot/mozilla/editor/libeditor/text/nsTextEditRulesBidi.cpp,v  <--  nsTextEditRulesBidi.cpp
new revision: 1.19; previous revision: 1.18
Checking in editor/libeditor/text/nsTextEditRules.h;
/cvsroot/mozilla/editor/libeditor/text/nsTextEditRules.h,v  <--  nsTextEditRules.h
new revision: 1.80; previous revision: 1.79
Checking in editor/libeditor/text/nsTextEditRules.cpp;
/cvsroot/mozilla/editor/libeditor/text/nsTextEditRules.cpp,v  <--  nsTextEditRules.cpp
new revision: 1.203; previous revision: 1.202
Checking in editor/libeditor/html/nsHTMLEditRules.cpp;
/cvsroot/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp,v  <--  nsHTMLEditRules.cpp
new revision: 1.323; previous revision: 1.322
Checking in layout/base/nsIFrameSelection.h;
/cvsroot/mozilla/layout/base/nsIFrameSelection.h,v  <--  nsIFrameSelection.h
new revision: 1.69; previous revision: 1.68
Checking in layout/base/nsCaret.cpp;
/cvsroot/mozilla/layout/base/nsCaret.cpp,v  <--  nsCaret.cpp
new revision: 1.153; previous revision: 1.152
Checking in layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.219; previous revision: 3.218
Closed: 15 years ago
Resolution: --- → FIXED
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.