330175 - Bidi: Visual caret movement during (logical) selection is confusing

Status: RESOLVED FIXED

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

Description

[Timwi]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

Normally Shift+Right Arrow means "select the next character that comes logically (not visually) after the cursor" (assuming I'm selecting from left to right.

However, if you have primarily Latin text but with Arabic text embedded in it, Shift+Right Arrow always selects visually to the right, which in Latin text means selecting the next character, in Arabic text means de-selecting the previous character, and at the boundary means select the whole Arabic text en masse.

Two reasons I think this should be changed:

(1) All native Windows controls behave the "logical" way (right-arrow means "select next character" when in LTR mode, even if it means the selection actually goes visually to the left). Try it in Notepad.

(2) In order to select an LTR word followed by an Arabic word, I have to: (a) use Shift+Right to select the LTR word, plus one character. This selects everything except for the last character of the Arabic word; (b) press Shift+Left to extend the selection to include the last Arabic character. By contrast, in Windows native controls I just keep pressing Shift+Right until everything is selected.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Uri Bernstein]

I was tempted to resolve this as a duplicate of bug 167288, but the truth is that the combination of visual caret movement and logical selection is really confusing, even when normal caret movemet is visual. OS X (which uses visual caret movement) solves this by reverting to logical movement when the shift key is down.

So issue 1, above, is Windows-specific and a duplicate of bug 167288, but I'm confirming this bug for issue 2. I'm changing the summary to reflect this.

There are two ways to fix this, IMO: one is to fix bug 195909 (re-implement visual selection), and the other (perhaps more realistic) is to go the OS X way and use logical movement while selecting.

Comment 2

[Simon Montagu :smontagu]

(In reply to comment #0)
> (2) In order to select an LTR word followed by an Arabic word, I have to: (a)
> use Shift+Right to select the LTR word, plus one character. This selects
> everything except for the last character of the Arabic word; (b) press
> Shift+Left to extend the selection to include the last Arabic character. By
> contrast, in Windows native controls I just keep pressing Shift+Right until
> everything is selected.

"Shift+Right [or Shift+Control+Right] until everything selected" works in Mozilla also, except in the case where the last word in the text control has the opposite directionality from the base direction, e.g. "english ARABIC" in an ltr control, where the caret gets stuck to the left of the C. Is this bug filed? It can be worked around by pressing Shift+End.

Having said that, I think the OSX behaviour described in comment 1 has a lot to be said for it.

Comment 3

[Uri Bernstein]

Attachment: patch

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 216580 [details] [diff] [review]
patch

+  PRInt32 mCaretMovementStyle;

PRUint8 please

Comment 5

[Uri Bernstein]

Checked in, with the change requested in comment #4.

Checking in modules/libpref/src/init/all.js;
/cvsroot/mozilla/modules/libpref/src/init/all.js,v  <--  all.js
new revision: 3.630; previous revision: 3.629
done
Checking in layout/base/nsIFrameSelection.h;
/cvsroot/mozilla/layout/base/nsIFrameSelection.h,v  <--  nsIFrameSelection.h
new revision: 1.70; previous revision: 1.69
done
Checking in layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.223; previous revision: 3.222
done
Checking in layout/generic/nsTextFrame.cpp;
/cvsroot/mozilla/layout/generic/nsTextFrame.cpp,v  <--  nsTextFrame.cpp
new revision: 1.567; previous revision: 1.566
done
Checking in layout/generic/nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.638; previous revision: 3.637
done