Closed Bug 327107 Opened 14 years ago Closed 14 years ago

After using backspace[delete] to remove a single LTR character in RTL text, the next press on backspace[delete] has no effect

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: uriber, Assigned: uriber)

References

Details

(Keywords: rtl, testcase)

Attachments

(2 files)

After using backspace to remove a single LTR character surrounded by RTL text (or at the end of an RTL paragraph, preceded by RTL text), pressing backspace again has no effect. Pressing backspace once more deletes the RTL character to the left of the caret.

The same is true when using "delete" instead of "backspace".
The same is also true after deleting a single RTL character in LTR text.
Attached file testcase
Self-documented.
This behaviour is consistent with the current specification of bidi caret movement. See http://www-306.ibm.com/software/globalization/topics/bidiui/multiline.jsp, under "Backspace and Delete".

To fix it, the spec can be amended in one of two ways:

1. In the section referred to above, in paragraph (b), remove the words "and this level has the same parity as the cursor level".

2. (Suggested by smontagu): add after "After the successful removal of a character by Delete or Backspace, the cursor level must be set to that of the removed character," something to the effect that if neither the character before nor the character after the cursor position is the same level (parity?) as the removed character, set the caret level to the level of the next logical character in the deletion direction.

(2) also covers the case of deleting a single level-2 character between level-1 and level-0 text, but is more difficult to implement, as getting the next logical character in the deletion direction is not trivial.
Keywords: testcase
Here's a case which would be fixed by the first solution, but not by the second:
1. Place the caret inside an RTL run (between two RTL characters).
2. Switch to an LTR keyboard layout.
3. Press "backspace".
The "backspace" in step three has no apparent effect.

Back when the caret level affected keyboard layout, step 3 would have the effect of switching the keyboard layout back to RTL. But with that feature disabled, the backspace does nothing.
This patch implements the first, simple, solution presented above.
Simon, given comment #3, perhaps we should just go with this?
Attachment #212412 - Flags: review?(smontagu)
Comment on attachment 212412 [details] [diff] [review]
patch (alternative 1)

r=me, noting that we really need to go through and look for all cases which don't make sense now that changing caret level doesn't change the keyboard input language.

Nit: while you're there, please make the spacing conform to local style, i.e. 
if (levelBefore == levelAfter)
Attachment #212412 - Flags: review?(smontagu) → review+
Comment on attachment 212412 [details] [diff] [review]
patch (alternative 1)

I'll fix the spacing when I check in, if/when I get sr.
Attachment #212412 - Flags: superreview?(roc)
Comment on attachment 212412 [details] [diff] [review]
patch (alternative 1)

rubberstamp!
Attachment #212412 - Flags: superreview?(roc) → superreview+
Checked in:
/cvsroot/mozilla/editor/libeditor/text/nsTextEditRulesBidi.cpp,v  <--  nsTextEditRulesBidi.cpp
new revision: 1.16; previous revision: 1.15
Assignee: mozilla → uriber
...and naturally I forgot to do the whitespace fix. I did it in a separate checkin.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment on attachment 212412 [details] [diff] [review]
patch (alternative 1)

People are complaining about this, so it would be nice to be able to tell them it'll be fixed in Fx 2.0.
Attachment #212412 - Flags: approval-branch-1.8.1?(roc)
Comment on attachment 212412 [details] [diff] [review]
patch (alternative 1)

I would like to wait for a month or so to see if there are any regressions before we land this on branch.
Attachment #212412 - Flags: approval-branch-1.8.1?(roc)
Depends on: 330457
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.