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
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.
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.
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
Status: NEW → ASSIGNED
Attachment #215176 - Flags: review?(smontagu)
Since most of the change in GetPrevNextBidiLevels() is indentation, here's apatch created with diff -w.
oops, forgot to include nsTextEditRules.h.
Take care of beginning/end of document (or, actually, beginning/end of scrollframe) cases.
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 done 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 done 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 done 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 done Checking in layout/base/nsIFrameSelection.h; /cvsroot/mozilla/layout/base/nsIFrameSelection.h,v <-- nsIFrameSelection.h new revision: 1.69; previous revision: 1.68 done Checking in layout/base/nsCaret.cpp; /cvsroot/mozilla/layout/base/nsCaret.cpp,v <-- nsCaret.cpp new revision: 1.153; previous revision: 1.152 done Checking in layout/generic/nsSelection.cpp; /cvsroot/mozilla/layout/generic/nsSelection.cpp,v <-- nsSelection.cpp new revision: 3.219; previous revision: 3.218 done
Status: ASSIGNED → RESOLVED
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.