1646669 - Change color of secondary cursor when text has both RTL and LTR text

Status: RESOLVED FIXED

(DevTools :: Source Editor, defect, P3)

Description

[pseudo_anon]

Attachment: Screenshot of duplicate cursor

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

Open devtools web console
Paste in some expression containing both RTL and LTR text (Example: encodeURIComponent("jבkאkמת!"))
Using the arrow keys, move along the bidirectional text

Actual results:

The cursor shown in the console duplicates and the two cursors move around, sitting on the edges where RTL meets LTR. This happens in both single-line and multi-line console.

Expected results:

Expected behavior would be a single cursor moving left or right along with the arrow keys pressed rather than the two individual cursors seen in the screenshot provided.

Not sure if this is a feature that's supposed to help visualize bidirectional text, but I couldn't easily find any info on such a feature, nor was there any visual indication of such a feature being active (other than the duplicate cursor).

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:nchevobbe, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Looks like a feature of CodeMirror (you can try to paste your example on the editor at https://codemirror.net/)
But on the example there, the second cursor is lighter.
Do you think we should do the same thing, or remove the second cursor completely ?

The other cursor has a CodeMirror-cursor CodeMirror-secondarycursor class, so it would be simple to style it differently or hide it.

Pinging itiel who might know what should be done here as well.

Comment 4

[Itiel]

I'm not as advanced developer as other RTL users might be, so it's hard for me to decide if having a second cursor in this specific case is useful or not, but I tend to think this is unnecessary. I could be wrong though.
Maybe Tomer or Yaron can help on this matter.

Comment 5

[Yaron Shahrabani]

I find it pretty helpful in cases where tags with RTL content are used in RTL context, for example:

<b>טקסט</b>

The logical movement of the cursor in this case is pretty confusing, if you copy this text to an RTL box and delete the characters from the beginning you'll see that the "digested" characters are different than the visual order.

I think that the lighter cursor solution is great, pretty self explanatory and pretty clear as opposed to the current solution.

Comment 6

[pseudo_anon]

(In reply to Yaron Shahrabani from comment #5)

I find it pretty helpful in cases where tags with RTL content are used in RTL context, for example:

<b>טקסט</b>

The logical movement of the cursor in this case is pretty confusing, if you copy this text to an RTL box and delete the characters from the beginning you'll see that the "digested" characters are different than the visual order.

I think that the lighter cursor solution is great, pretty self explanatory and pretty clear as opposed to the current solution.

In total agreement here. Some other visual indicator like a lighter cursor would be less confusing.
Glad this does not seem to be a bug.

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Let's keep this open so we can change the cursor color.

The color is set here: https://searchfox.org/mozilla-central/rev/c86c19bd64f8f19590a4190c282781d3a9631422/devtools/client/shared/sourceeditor/codemirror/lib/codemirror.css#51-54 , but is probably overridden by higher specificity rules (maybe https://searchfox.org/mozilla-central/source/devtools/client/themes/light-theme.css#189 and https://searchfox.org/mozilla-central/source/devtools/client/themes/dark-theme.css#198)

We might want to change those rules and add a :not(.CodeMirror-secondarycursor) so we keep a distinct color for the helpful secondary cursor.

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1646669 - Fix CodeMirror secondary cursor color. r=itiel.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0df4f44fad77
Fix CodeMirror secondary cursor color. r=Itiel.

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/0df4f44fad77