Closed Bug 1638240 Opened 4 years ago Closed 4 years ago

Direction of text selection is broken for right-to-left documents on Mac

Categories

(Core :: Layout, defect, P2)

76 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1644511
Tracking Status
firefox-esr68 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- affected

People

(Reporter: bashar.harfoush, Assigned: saschanaz)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:76.0) Gecko/20100101 Firefox/76.0

Steps to reproduce:

I created a contenteditable div with dir="rtl". I tried selecting a piece of text using shift-arrow but the direction of selection is reversed. In the middle of an Arabic(RTL) text, if I press shift-left to select a piece of text, Firefox will select the text on the right and vice-versa. A demonstration can be seen here: https://codepen.io/basharh/pen/rNOZKOL

Actual results:

The text was selected in the reverse direction of the arrow key that was pressed as part of shift-arrowkey.

Expected results:

The text should be selected in the right direction of the arrow key that was pressed as part of shift-arrowkey.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → DOM: Editor
Product: Firefox → Core

Can also be reproduced on Ubuntu 20.04.

Interestingly it selects in the right direction on Windows, while it still puts caret in a wrong position: Select by Shift+ArrowRight and tap ArrowRight to cancel selection, and you'll get the caret in the left side of the selection instead of right side.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Severity: -- → S3

This looks important for RTL language users, should I set this S2?

Yes, I think it deserves a higher severity. Is this a new regression?

The Windows bug has been their for years. Not sure about the non-Windows part yet.

Severity: S3 → S2

Kagami, can you help us to figure where goes wrong? Thank you.

Flags: needinfo?(krosylight)
Assignee: nobody → krosylight
Flags: needinfo?(krosylight)

Hey all, thank you for looking into this. I've thought to compile a list of more arrow and arrow+select behaviors and how they currently work on Firefox:

  • Option+Arrow: Works
  • Option+Shift+Arrow: Does not work(direction reversed)
  • Command+Arrow: Does not work(direction reversed)
  • Command+Shift+Arrow: Does not work(direction reversed)

Turns out that there is a flag bidi.edit.caret_movement_style to control this behavior, introduced by Bug 330175. The current default is "Logical for text selection, visual otherwise", and the reason was to match the Windows behavior at that time (even when our current Windows implementation never follows the flag). IMO this is not true as Notepad and EdgeHTML on Windows do "visual movement" on RTL texts.

I'm not quite sure why only Windows does not follow that flag. It currently hits nsFrameSelection::PhysicalMove which always forces "visual movement", and seemingly only on Windows. Maybe it hits ::CharacterMove on other platforms?

Mirko, could you confirm whether Shift+Arrow on Linux hits ::PhysicalMove or not?

Flags: needinfo?(mbrodesser)

(In reply to Kagami :saschanaz from comment #8)

Turns out that there is a flag bidi.edit.caret_movement_style to control this behavior, introduced by Bug 330175. The current default is "Logical for text selection, visual otherwise", and the reason was to match the Windows behavior at that time (even when our current Windows implementation never follows the flag). IMO this is not true as Notepad and EdgeHTML on Windows do "visual movement" on RTL texts.

I'm not quite sure why only Windows does not follow that flag. It currently hits nsFrameSelection::PhysicalMove which always forces "visual movement", and seemingly only on Windows. Maybe it hits ::CharacterMove on other platforms?

Mirko, could you confirm whether Shift+Arrow on Linux hits ::PhysicalMove or not?

nsFrameSelection::PhysicalMove is not hit on Ubuntu. nsFrameSelection::CharacterMove is:

Thread 1 "firefox" hit Breakpoint 2, nsFrameSelection::CharacterMove (this=0x7fff968fa010, aForward=true, aExtend=true)
    at /home/mirko/src/firefox/gecko/layout/generic/nsFrameSelection.cpp:2071
2071	  return MoveCaret(aForward ? eDirNext : eDirPrevious, aExtend, eSelectCluster,
(gdb) up
#1  0x00007fffe371b6e8 in mozilla::PresShell::CharacterMove (this=0x7fffae558000, aForward=true, aExtend=true)
    at /home/mirko/src/firefox/gecko/layout/base/PresShell.cpp:2284
2284	  return frameSelection->CharacterMove(aForward, aExtend);
(gdb) up
#2  0x00007fffdfd4be1c in nsSelectCommand::DoCommand (this=0x7fff8d9dc0a0, aCommandName=0x7fffd77aea1f "cmd_selectCharNext", 
    aCommandContext=0x7fff990f5c10) at /home/mirko/src/firefox/gecko/dom/base/nsGlobalWindowCommands.cpp:420
420	      return (selCont->*(selectCommands[i].select))(forward, true);
Flags: needinfo?(mbrodesser)

Thanks! Very interesting, I wonder what causes the difference.

My second question is whether we still want to keep this behavior. Since the original intent was to match the old Windows behavior (I'm not sure how Windows really did, though) and now the Windows behavior matches with other browser behaviors, maybe we should drop the flag or at least change the default value?

Flags: needinfo?(mbrodesser)
Flags: needinfo?(masayuki)

(In reply to Kagami :saschanaz from comment #11)

My second question is whether we still want to keep this behavior. Since the original intent was to match the old Windows behavior (I'm not sure how Windows really did, though) and now the Windows behavior matches with other browser behaviors,

With which browsers on which platforms did you test the behavior? Safari/OS X and Chrome on OS X, Linux and Windows would be interesting. But even then we should be careful about following their behavior. Perhaps it's a first step to gather more insights.

maybe we should drop the flag or at least change the default value?

Let's first gather more insights. Unless Masayuki has a clearer picture about this. Perhaps Thunderbird-developers have useful knowledge about this too.

With which browsers on which platforms did you test the behavior?

  • Firefox, Windows: Typical visual direction
  • Firefox, Linux: Logical direction (visually reversed)
  • Chrome, Windows: Typical visual direction
  • Chrome, Linux: Typical visual direction
  • Gnome Web (WebKit), Linux: Logical direction (visually reversed)
  • IE/EdgeHTML: Typical visual direction

I have no macOS so I couldn't test there.

Did you verify the Windows' standard behavior on Win7, Win 8.1 too? If they are different from Win10, I think that we should change our behavior only on Win10, etc.

Flags: needinfo?(masayuki)

(In reply to Kagami :saschanaz from comment #13)

With which browsers on which platforms did you test the behavior?

  • Firefox, Windows: Typical visual direction
  • Firefox, Linux: Logical direction (visually reversed)
  • Chrome, Windows: Typical visual direction
  • Chrome, Linux: Typical visual direction
  • Gnome Web (WebKit), Linux: Logical direction (visually reversed)
  • IE/EdgeHTML: Typical visual direction

I have no macOS so I couldn't test there.

We should clarify the scope if this ticket. The reporter filed the ticket for OS X. If Gnome Web behaves the same like Safari on OS X, Firefox should perhaps 'inherit' Safari's behavior on OS X.

I can confirm that Safari and Safari Technology Preview on macOS use visual direction for cursor movement and selection with the keyboard in that editable div. (Chrome on macOS does the same.)

Firefox Nightly on macOS uses visual direction for cursor movement and logical direction for selection.

I feel we need more input from actual RTL language users about this. (One from the reporter, thank you!)

See Also: → 1644489
Depends on: 1644511

(In reply to Kagami :saschanaz from comment #17)

I feel we need more input from actual RTL language users about this. (One from the reporter, thank you!)

Hey flod, I am not sure if you're the one to ask, but still wanna try. Do you know if we can get more input from RTL users for this? Thank you.

Flags: needinfo?(francesco.lodolo)

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #18)

(In reply to Kagami :saschanaz from comment #17)

I feel we need more input from actual RTL language users about this. (One from the reporter, thank you!)

Hey flod, I am not sure if you're the one to ask, but still wanna try. Do you know if we can get more input from RTL users for this? Thank you.

I CCed a few folks working on RTL localizations (Hebrew, Persian), hopefully they can chime in and reach out to other folks in their communities.

Flags: needinfo?(francesco.lodolo)

I'm on Windows 10 and can't reproduce the issue on comment 0. Also Ctrl+Arrow and Ctrl+Shift+Arrow seem to work as intended. IIRC the same is true for both on Windows 7. I can test to confirm the latter if that's important.

(In reply to Kagami :saschanaz from comment #2)

Interestingly it selects in the right direction on Windows, while it still puts caret in a wrong position: Select by Shift+ArrowRight and tap ArrowRight to cancel selection, and you'll get the caret in the left side of the selection instead of right side.

Interesting. For years I also had this issue, but treated it as a minor annoyance and didn't actually consider it as an issue. But yes, I'd expect hitting ArrowRight to cancel the selection in this case.

I'd like to understand what is the difference between logical and visual direction?

IIRC the same is true for both on Windows 7. I can test to confirm the latter if that's important.

Yes, please test how Firefox and other browsers behave on Windows 7. Thanks!

Interesting. For years I also had this issue, but treated it as a minor annoyance and didn't actually consider it as an issue. But yes, I'd expect hitting ArrowRight to cancel the selection in this case.

It's fixed in today's build, please try it and share your thoughts 😀

I'd like to understand what is the difference between logical and visual direction?

"Visual" direction is the direction you see on your display, while "logical" direction is that "next" maps to "right" and "previous" maps to "left".

Also CC'ing :jfkthame who recently reviewed my bidi patch.

(In reply to Kagami :saschanaz from comment #21)

IIRC the same is true for both on Windows 7. I can test to confirm the latter if that's important.

Yes, please test how Firefox and other browsers behave on Windows 7. Thanks!

The following applies to Windows 7 and Windows 10, on latest Nightly, tested with the demo from comment 0:
Shift+Arrow using RTL characters- works as intended
Ctrl+Shift+Arrow using RTL characters- works as intended
Selecting RTL text and then pressing ArrowRight- moves the caret to the right of the selection, as intended
Shift+Arrow using LTR characters- works as intended
Ctrl+Shift+Arrow using LTR characters- works as intended
Selecting LTR text and then pressing ArrowRight- moves the caret to the left of the selection, NOT as intended (for the average user, at least). This is a regression, presumably from bug 1644511 as it works on yesterday's build.
I'm not sure if something should be done about it, as logically ArrowRight should go to the previous character because we're in RTL context, thus going to the left on LTR text. But it'll confuse average users for sure. Me included.
The issue can be seen on LTR context, with RTL text.

On Google Chrome on Windows 7 (haven't tested Windows 10, I assume the results would be identical) and the new Edge on Windows 10:
Shift+Arrow using RTL characters- works as intended
Ctrl+Shift+Arrow using RTL characters- works as intended
Selecting RTL text and then pressing ArrowRight- moves the caret to the right of the selection, as intended
Shift+Arrow using LTR characters- Selection is inverted to the other side. NOT as intended for average users
Ctrl+Shift+Arrow using LTR characters- Selection is inverted to the other side. NOT as intended for average users
Selecting LTR text and then pressing ArrowRight- works actually as intended! So maybe we can do better here.

I've seen comment 7 mentions Option+Arrow, unsure what that means.

Interesting. For years I also had this issue, but treated it as a minor annoyance and didn't actually consider it as an issue. But yes, I'd expect hitting ArrowRight to cancel the selection in this case.

It's fixed in today's build, please try it and share your thoughts 😀

Mentioned above.

I can see odd issues when selecting mixed LTR and RTL characters, but that's out of scope for this bug, I guess.

Kagami, would you like more tests to be done? Just name it.

Flags: needinfo?(krosylight)

Selecting LTR text and then pressing ArrowRight- moves the caret to the left of the selection, NOT as intended (for the average user, at least). This is a regression, presumably from bug 1644511 as it works on yesterday's build.

Great catch, I'll look into it. (Edit: Probably because GetCaretDirection refers paragraph direction instead of each text direction.)

Kagami, would you like more tests to be done? Just name it.

Could you also test IE and the old Edge (oops no such thing sorry) on Windows 7? Thanks!

Flags: needinfo?(krosylight) → needinfo?(itiel_yn8)

(In reply to Kagami :saschanaz from comment #23)

Selecting LTR text and then pressing ArrowRight- moves the caret to the left of the selection, NOT as intended (for the average user, at least). This is a regression, presumably from bug 1644511 as it works on yesterday's build.

Great catch, I'll look into it. (Edit: Probably because GetCaretDirection refers paragraph direction instead of each text direction.)

Thanks!

Kagami, would you like more tests to be done? Just name it.

Could you also test IE and the old Edge (oops no such thing sorry) on Windows 7? Thanks!

Sure!

Internet Explorer on Windows 7:
Shift+Arrow using RTL characters- works as intended
Ctrl+Shift+Arrow using RTL characters- works as intended
Selecting RTL text and then pressing ArrowRight- moves the caret to the right of the selection, as intended
Shift+Arrow using LTR characters- Selection is inverted to the other side. NOT as intended for average users
Ctrl+Shift+Arrow using LTR characters- Selection is inverted to the other side. NOT as intended for average users
Selecting LTR text and then pressing ArrowRight- moves the caret to the left of the selection, NOT as intended

Flags: needinfo?(itiel_yn8)

Thank you for testing IE! So the RTL text selection behaviors on each OS are:

Windows: All browsers selects in visual direction
macOS: All browsers selects in visual direction except Firefox
Linux: Chrome selects in visual direction while Firefox and Gnome Web does in logical direction

...with LTR-in-RTL selection behavior mixed. I think its safe to change the behavior on macOS, but what do you all think about Linux? Should we also follow Chrome there?

I can see odd issues when selecting mixed LTR and RTL characters, but that's out of scope for this bug, I guess.

Yes, it's odd and I think correcting the behavior requires multi-range selection, which currently is rejected by the Selection API spec.

Interestingly people on Bug 167288 argue that they need logical direction for selection.

Depends on: 1645879
Depends on: 1646240

Since this depends on layout component:

Component: DOM: Editor → Layout

:ehsan, I heard you might have some opinion too. Do you think that we have to keep the reversed direction on Linux or that we should follow Chrome?

Flags: needinfo?(ehsan)
Priority: -- → P2

Kagami, could you clarify whether this is a regression or not? (or maybe was there a regression that made this easier to hit, or something like that?)

Comment 22 and 23 suggest that there is a regression here; but the fact that you duped bug 167288 to this bug (an 18-year-old bug :D ) suggests that this would very much not be a regression...

Flags: needinfo?(krosylight)

Ah, that's Bug 1645879 caused by Bug 1644511.

Flags: needinfo?(krosylight)

Sorry, I'm still not clear on what comment 31 means in terms of answering the questions in comment 30 is - could you clarify whether this is a regression and/or if this is the same as bug 167288? (I don't think both of those things can be true simultaneously, though maybe there's some subtlety.)

(motivation: this bug came up in layout bug triage, and we're trying to reason about the appropriate priority/severity to set, & what versions are affected.)

Flags: needinfo?(krosylight)

I mean, comment #22 tried the fix from Bug 1644511 (which is implied in comment #21) and found a regression from the bug. It's a separate regression not directly related here. In other words, this bug is not a regression.

Sorry for confusion 😅

Flags: needinfo?(krosylight)
Summary: Direction of text selection is broken for right-to-left documents → Direction of text selection is broken for right-to-left documents on Mac

Hi Jens,

Since there was no complaint about the new Nightly behavior (RTL text selection in visual direction via keyboard), I intend to ship this to release branches. But as I'm not an RTL language user I want to get a final double check, do you have some idea who I could contact?

Flags: needinfo?(jstutte)

(In reply to Kagami :saschanaz from comment #34)

Hi Jens,

Since there was no complaint about the new Nightly behavior (RTL text selection in visual direction via keyboard), I intend to ship this to release branches. But as I'm not an RTL language user I want to get a final double check, do you have some idea who I could contact?

Hello Kagami,

I might be able to help with the testing. This what I see in Nightly now:

Option+Arrow: Works
Option+Shift+Arrow: Works (this wasn't working when I originally filed the bug)
Command+Arrow: Does not work(direction reversed)
Command+Shift+Arrow: Does not work(direction reversed)

On Mac, Ctrl+Arrow doesn't seem to allow you to navigate in the text which is not a problem except that it triggers movement only when shift is pressed the same following behavior is exhibited in Chrome btw). This is the current behavior(which might be what is intended?)

  • Ctrl+Arrow: No movement
  • Ctrl+Shift+Arrow: Move and Select till the end of line

Thanks Bashar!

It seems Command+Arrow is to jump to the beginning/end of the line, am I correct? If then:

I guess Command+LeftArrow currently is mapped to cmd_beginLine and +RightArrow as cmd_endLine. Intuitively cmd_beginLine should go to the "beginning" of the line and cmd_endLine key should go to the "end" of the line, and thus the current behavior makes sense on platforms where the corresponding shortcuts are Home/End keys.

But since macOS uses arrows to move to line edge, It seems we should define new commands to correctly handle those macOS shortcuts. Masayuki, do you have some thoughts?

(BTW, This file does not define such commands, which file defines them on Mac?)

Flags: needinfo?(masayuki)

Yeah, in my environment, Cmd + ArrowRight is mapped to moveToRightEndOfLine: command, and Cmd + ArrowLeft is mapped to moveToLeftEndOfLine. The names contain physical direction, but we map them to logical direction commands. So, we need to remap them and similar commands to physical direction commands. But looks like that there is no proper command for them. Perhaps, cmd_leftLineEnd, cmd_rightLineEnd and related commands should be created and implemented.

Flags: needinfo?(masayuki)

Hi, sorry for being late, but I assume you already had the help needed?

Flags: needinfo?(jstutte)

Exactly 👍

Depends on: 1665333

Hi Kagami: I'm just doing an audit of open S2 bugs in Layout. Is this now shipped to release? Does this bug need to remain open? (And if so, is it still and S2? Testing locally in Nightly seems correct for selection but I still see the issue for caret movement using Cmd + ArrowRight/Left.)

Flags: needinfo?(krosylight)

Hmm, maybe we can close this as a dupe of bug 1644511 and track remaining Cmd thing in bug 1665333.

Flags: needinfo?(krosylight)
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Flags: needinfo?(ehsan.akhgari)
You need to log in before you can comment on or make changes to this bug.