Page MenuHomePhabricator

Support CodeMirror syntax highlighting on RTL wikis
Open, Stalled, MediumPublic8 Story Points

Description

From Eranroz, on Meta talk page:

For right to left languages it is broken, for example the text direction is always left to right and browser switch text direction functionality ([Ctrl+Shift/Ctrl+Shift+X]) doesn't change the text direction compared to simple textarea. This is very important to RTL users as it is quite common to handle LTR embedded text within RTL content.

Relevant links:

Details

Related Gerrit Patches:
mediawiki/extensions/CodeMirror : masterAdd RTL support in VE source editor mode

Event Timeline

DannyH created this task.Jul 7 2017, 5:18 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 7 2017, 5:18 PM
IKhitron added a subscriber: IKhitron.
DannyH triaged this task as Medium priority.Jul 7 2017, 5:23 PM
DannyH updated the task description. (Show Details)Jul 7 2017, 5:39 PM
DannyH updated the task description. (Show Details)Jul 7 2017, 5:48 PM
mxn added a subscriber: mxn.Jul 10 2017, 8:37 AM

When I test at https://he.wikipedia.beta.wmflabs.org/w/index.php?title=%D7%A2%D7%AA%D7%99%D7%94_%D7%96%D7%A8&action=edit, inserting the cursor within highlighted text doesn't work right. The cursor typically appears in the wrong spot. Also if I insert the cursor within RTL text and start typing, the cursor splits (the position of the CodeMirror cursor is different than the position of the underlying textarea cursor).

kaldari set the point value for this task to 8.Jul 12 2017, 12:19 AM
kaldari moved this task from To be estimated/discussed to Estimated on the Community-Tech board.
kaldari added a subscriber: Mooeypoo.
Restricted Application added a project: I18n. · View Herald TranscriptJul 18 2017, 11:20 PM

Apparently, the split cursor behavior is intended: https://github.com/codemirror/CodeMirror/issues/2802

Ah. I am going to grab Moriel and talk about this.

It looks like there is definitely a cursor placement bug when you click on the 2nd line of text, but it seems to be a bug in the CodeMirror library, not the MediaWiki extension. You can test at https://codemirror.net/demo/bidi.html. We should report this bug upstream.

@kaldari I talked to Moriel and she thinks there are a few bugs in how we handle RTL as well as the library demo page. The NWE takes care of those because VE has inbuilt support for cursor handling when it comes to RTL (with mixed in LTR) text. She thinks it's a crazy thing to be doing by ourselves, we should rely on the editor to do it and not try to override it.

Niharika updated the task description. (Show Details)Jul 21 2017, 12:16 AM

@Pastakhov Can you tell us why we might be overriding the cursor movement over the default one? Is it possible to turn off cursor movements as handled by the Codemirror library and use the ones from the default editor?

Pastakhov added a comment.EditedJul 21 2017, 11:05 AM

@Niharika Maybe it is possible by process event:

"cursorActivity" (instance: CodeMirror)
    Will be fired when the cursor or selection moves, or any change is made to the editor content.
Niharika moved this task from Ready to In Development on the Community-Tech-Sprint board.
Niharika added a comment.EditedJul 22 2017, 12:29 AM

@Pastakhov Why is Codemirror not being loaded via composer?

I think the latest version of the library has all the fixes we need here. I want to try updating it.

@Niharika It is easier to install the extension without composer usage and maybe when I created the extension composer was not widely used for mediawiki extensions (not sure).

And I think it is good idea to use composer there.

Okay, so here's what I know:

  • CodeMirror does a poor job of supporting RTL
  • The latest branch is better but far from perfect
  • Some old branches of code exist aimed at improving RTL support but they're very out of date and would need tests and more to get close to being merge-able
  • CodeMirror does not have an option to let the browser take care of the cursor events which is a bummer

Open question: The library author has commented in a few issues that if someone wants him to prioritize something, we can fund him for it. Do we want to consider this approach?

Niharika updated the task description. (Show Details)Jul 31 2017, 1:43 PM

Change 371713 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/CodeMirror@master] Add RTL support in VE source editor mode

https://gerrit.wikimedia.org/r/371713

As VE doesn't use CodeMirror for input (and does use native browser cursoring in its own CE) this won't be a problem for the new wikitext editor :)

As VE doesn't use CodeMirror for input (and does use native browser cursoring in its own CE) this won't be a problem for the new wikitext editor :)

@Esanders Do you think it's possible for us to take away cursor control from CM and let the browser handle it for the classic/advanced editors in the way that VE does it? CM doesn't provide a config option for that in the library and insists on handling cursor movements itself, which it is not very good at.

Filled an upstream request at https://github.com/codemirror/CodeMirror/issues/4921 to make the cursor control in CodeMirror configurable.

Change 371713 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] Add RTL support in VE source editor mode

https://gerrit.wikimedia.org/r/371713

Some RTL bugs may have been fixed by https://gerrit.wikimedia.org/r/#/c/380678/. Let's re-test and see if there are still problems.

Some RTL bugs may have been fixed by https://gerrit.wikimedia.org/r/#/c/380678/. Let's re-test and see if there are still problems.

Well, I tried to check this on testwiki, using javascript, but still on right aligned test, if pressing keyboard End key, the cursor remains at the start of the line.

Some RTL bugs may have been fixed by https://gerrit.wikimedia.org/r/#/c/380678/. Let's re-test and see if there are still problems.

Well, I tried to check this on testwiki, using javascript, but still on right aligned test, if pressing keyboard End key, the cursor remains at the start of the line.

You didn't have to do that. Can you instead test on https://he.wikipedia.beta.wmflabs.org/wiki/יוסף_הלוי (make sure your JavaScript doesn't interfere) and tell us what are the major problems with it? It seems a little better than before but I think I still see some problems.

The goal is to figure out how disruptive the bugs are. Is it something that will stop people from editing completely? Is it only a bug that occurs sometimes? Is it something that only occurs when editing templates? Etc.

You didn't have to do that. Can you instead test on https://he.wikipedia.beta.wmflabs.org/wiki/יוסף_הלוי (make sure your JavaScript doesn't interfere) and tell us what are the major problems with it? It seems a little better than before but I think I still see some problems.

Thank you for this link. I tried, and it's even worse than with javascript today. The problems are:

  1. Ctrl Shift and Alt Shift do not work,
  2. The cursor position does not fit the real editing position.
  3. You can't put the cursor to any place with a mouse.
  4. The line beginning is still on the opposite side.
  5. Pressing the Home or End keyboard keys puts the cursor to different line of the document.
  6. Etc.

The good thing is that now, in opposite to a month before, you can work on rtl alignment, with rtl text only, and see the colours.
Sorry, but it still can't be used. Thank you again.

Thanks @IKhitron. We'll keep investigating.

Epine added a subscriber: Epine.Jan 27 2018, 10:30 AM
Amire80 moved this task from Untriaged to RTL on the I18n board.Feb 3 2018, 2:36 PM
TBolliger moved this task from Estimated to Archive on the Community-Tech board.Feb 14 2018, 12:58 AM
TBolliger renamed this task from Syntax highlighting - RTL problems to Support CodeMirror syntax highlighting on RTL wikis.Mar 2 2018, 12:49 AM
TBolliger removed Niharika as the assignee of this task.Mar 2 2018, 12:51 AM

Marking as Stalled due to CodeMirror library work required by external developers.

TBolliger changed the task status from Open to Stalled.Apr 11 2018, 10:54 PM

@Esanders Do you think it's possible for us to take away cursor control from CM and let the browser handle it for the classic/advanced editors in the way that VE does it? CM doesn't provide a config option for that in the library and insists on handling cursor movements itself, which it is not very good at.

That would be feasible, although with T190720 you may lose IE11 support.

Another issue might be scroll sync, as on some devices there is a hardware accelerated scrolling animation that may not fire enough events to keep the two editors in sync during the animation: http://jsfiddle.net/5kth4fnx/47/

@Esanders Do you think it's possible for us to take away cursor control from CM and let the browser handle it for the classic/advanced editors in the way that VE does it? CM doesn't provide a config option for that in the library and insists on handling cursor movements itself, which it is not very good at.

That would be feasible, although with T190720 you may lose IE11 support.

Making the feature available to some is better than making it available to none. Also with T191093, it looks like we might have to drop support for IE and Edge altogether.

Another issue might be scroll sync, as on some devices there is a hardware accelerated scrolling animation that may not fire enough events to keep the two editors in sync during the animation: http://jsfiddle.net/5kth4fnx/47/

That does not sound pleasant. How does VE take care of that?