Page MenuHomePhabricator

Enable CodeMirror in 2017 Wikitext editor in all LTR without the beta feature
Closed, ResolvedPublic

Event Timeline

TBolliger triaged this task as Medium priority.Apr 10 2018, 9:30 PM
TBolliger created this task.

It can be done immediately. :)
We need to enable Codemirror for RTL wikis but disable it for the 2010 and 2006 editors. Right now there is only one config flag that turns on the beta feature CodeMirrorBetaFeature so this would involve creating another to turn it off on the classic editors.

@Niharika — I haven't tested yet, but is CodeMirror in 2017 WTE on RTL language wikis functional? Bug-free?

We could just hard-code in to CodeMirror's code for the 2010/2006/2003 editors that it's disabled on RTL wikis, and then enable everywhere-everywhere, and so the 2017 editor would "just work"?

@Niharika — I haven't tested yet, but is CodeMirror in 2017 WTE on RTL language wikis functional? Bug-free?

I haven't tried it again recently but from a testing I did with Moriel months back, it was much better on 2017 WTE. CodeMirror doing the cursor handling is what causes most of the CodeMirror bugs in RTL. 2017 WTE takes that over and hands it to the browser.
You can give it a go here.

We could just hard-code in to CodeMirror's code for the 2010/2006/2003 editors that it's disabled on RTL wikis, and then enable everywhere-everywhere, and so the 2017 editor would "just work"?

I didn't think you'd be the one suggesting putting in hacks, James!
I think adding a config flag would be cleaner and not much additional work, honestly. We won't be breaking things randomly for a third party wiki either (if one uses Codemirror). :)

You asked me a couple to check CodeMirror on RTL, so I did this now again. I can confirm, after the check on mobile, that it looks ways better here and ready for deployment. But I also need to check it on a desktop computer tomorrow, because it can be different there. My main concern is Alt-Shift. If it works, everything is fine. I'll be back.

Well, I tried on desktop computer, but it does not work. I get all the time error 503 "server problem on loading, try again?".

Tried again. It works on desktop, but Alt-Shift does not work. I really think you should spent 5 minutes and a direction button. Something as one of the buttons in"BiDiEditing" gadget on meta.

Tried again. It works on desktop, but Alt-Shift does not work.

Is it any more or less broken than VE (in either visual or source mode)?

I really think you should spent 5 minutes and a direction button. Something as one of the buttons in"BiDiEditing" gadget on meta.

There is a direction button in VE (under the hamburger menu / alt+shift+x), although testing it now it appears the CM functionality has regressed.

Change 426951 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/CodeMirror@master] Use CM API for setting document direction

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

Tried again. It works on desktop, but Alt-Shift does not work. I really think you should spent 5 minutes and a direction button. Something as one of the buttons in"BiDiEditing" gadget on meta.

Alt-Shift or Ctrl-Shift? On Windows Alt-Shift changes the keyboard. You probably want Ctrl-Shift-X (for Firefox) or Ctrl-Shift (on IE and Chrome).

I really think you should spent 5 minutes and a direction button. Something as one of the buttons in"BiDiEditing" gadget on meta.

There is a direction button in VE (under the hamburger menu / alt+shift+x), although testing it now it appears the CM functionality has regressed.

The usual VE menu item and keyboard shortcut do the right thing. They should work in all modes uniformly. (If it's possible to not override what the browsers do, it's even better, but if it has to be overridden, then the current VE behavior is OK.)

We could just hard-code in to CodeMirror's code for the 2010/2006/2003 editors that it's disabled on RTL wikis, and then enable everywhere-everywhere, and so the 2017 editor would "just work"?

I didn't think you'd be the one suggesting putting in hacks, James!
I think adding a config flag would be cleaner and not much additional work, honestly. We won't be breaking things randomly for a third party wiki either (if one uses Codemirror). :)

We are only going to present one beta feature to the user so we will have to explain RTL wikis that the feature is editor dependent anyway. Having the editor use feature detection to work out if it can load a plugin seems cleaner. Third party wikis will have their existing config respected, and won't have to migrate any settings, or enable/disable another flag.

Is it any more or less broken than VE (in either visual or source mode)?

Sorry? I tried it in VE, in source mode.

There is a direction button in VE (under the hamburger menu / alt+shift+x), although testing it now it appears the CM functionality has regressed.
Alt-Shift-x is special:randompage

Alt-Shift or Ctrl-Shift? On Windows Alt-Shift changes the keyboard. You probably want Ctrl-Shift-X (for Firefox) or Ctrl-Shift (on IE and Chrome).

Of course Ctrl-shift. Thanks.

The usual VE menu item and keyboard shortcut do the right thing. They should work in all modes uniformly. (If it's possible to not override what the browsers do, it's even better, but if it has to be overridden, then the current VE behavior is OK.)

See above.

We could just hard-code in to CodeMirror's code for the 2010/2006/2003 editors that it's disabled on RTL wikis, and then enable everywhere-everywhere, and so the 2017 editor would "just work"?

I didn't think you'd be the one suggesting putting in hacks, James!
I think adding a config flag would be cleaner and not much additional work, honestly. We won't be breaking things randomly for a third party wiki either (if one uses Codemirror). :)

We are only going to present one beta feature to the user so we will have to explain RTL wikis that the feature is editor dependent anyway. Having the editor use feature detection to work out if it can load a plugin seems cleaner. Third party wikis will have their existing config respected, and won't have to migrate any settings, or enable/disable another flag.

To clarify, with this ticket the beta feature will no longer be presented. This ticket also involves graduating syntax highlighting out of beta.
I will defer to your judgement on the implementation. I was thinking of a config to enable codemirror on 2017 WTE, enabled everywhere, independent of the config for classic editor which will be on for ltr wikis only.

There is a direction button in VE (under the hamburger menu / alt+shift+x), although testing it now it appears the CM functionality has regressed.
Alt-Shift-x is special:randompage

I believe you meant Ctrl-Shift-x. It works weird. The text remains without change, but whrn you choose it with the mouse, it becomes ltr for the second.

There is a direction button in VE (under the hamburger menu / alt+shift+x), although testing it now it appears the CM functionality has regressed.
Alt-Shift-x is special:randompage

I believe you meant Ctrl-Shift-x

It varies depending on your OS

It works weird. The text remains without change, but whrn you choose it with the mouse, it becomes ltr for the second.

Yes, that was the regression I was referring to. It is fixed by the patch above.

@Deskana I'm thinking of expanding the scope of this ticket to make syntax highlighting available on 2017 editor, for both LTR and RTL wikis (without a beta feature). The only one remaining bug that I can reliably reproduce in the 2017 editor is T191093: In Edge Legacy, on first load CodeMirror jumps cursor to beginning of article. If we aren't able to fix it, we'll probably end up disabling syntax highlighting on IE & Edge completely.
This would probably just mean making 2017WTE loading of syntax highlighting independent of the beta feature config flag.
Thoughts?

Change 427352 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/CodeMirror@master] Never load ext.CodeMirror on RTL wikis

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

@Deskana I'm thinking of expanding the scope of this ticket to make syntax highlighting available on 2017 editor, for both LTR and RTL wikis (without a beta feature). […] Thoughts?

Given that the 2017 wikitext editor is an opt-in beta on its own, I think that would be fine. If people experience issues with the syntax highlighter, they can turn it off and report them.

Niharika renamed this task from Enable CodeMirror for RTL in 2017 Wikitext editor to Enable CodeMirror in 2017 Wikitext editor in all wikis without the beta feature.Apr 18 2018, 5:25 PM

Change 427352 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] Never load ext.CodeMirror on RTL pages

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

Change 427457 had a related patch set uploaded (by Jforrester; owner: Niharika29):
[mediawiki/extensions/CodeMirror@master] Load CodeMirror with VE always

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

Change 427457 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] Load CodeMirror with VE always

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

Change 428968 had a related patch set uploaded (by Niharika29; owner: Niharika29):
[operations/mediawiki-config@master] Graduate CodeMirror from beta on 2017 Wikitext Editor for all wikis

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

Change 426951 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] Use CM API for setting document direction

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

Will this be deployed next week, to all wikis? Not as a beta feature? (Or first as a beta feature?)

In T191923#4161327, @Johan wrote:

Will this be deployed next week, to all wikis? Not as a beta feature? (Or first as a beta feature?)

  • It's currently a Beta Feature enabled on all wikis LTR-content wikis but not RTL-content wikis.
  • If you enable the Beta Feature, it makes the tool available but not enabled; you then have to enable it inside the editor (and can disable it from there too).
  • As of later today, the Beta Feature will be available on RTL-content wikis too (but it will only operate in the 2017 wikitext editor).
  • As of next week's train, the feature will always be available in the 2017 wikitext editor (available but not by default enabled).

Change 428968 merged by jenkins-bot:
[operations/mediawiki-config@master] Enable CodeMirror on RTL wikis

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

Deskana claimed this task.

Change 429850 had a related patch set uploaded (by Niharika29; owner: Esanders):
[mediawiki/extensions/CodeMirror@wmf/1.32.0-wmf.1] Use CM API for setting document direction

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

Change 429850 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@wmf/1.32.0-wmf.1] Use CM API for setting document direction

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

Mentioned in SAL (#wikimedia-operations) [2018-04-30T18:17:39Z] <catrope@tin> Synchronized php-1.32.0-wmf.1/extensions/CodeMirror/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js: T191923 (duration: 01m 00s)

Reopen the task as the mission as not yet completed: "Enable CodeMirror in 2017 Wikitext editor in all wikis without the beta feature" means:

TBolliger renamed this task from Enable CodeMirror in 2017 Wikitext editor in all wikis without the beta feature to Enable CodeMirror in 2017 Wikitext editor in all LTR without the beta feature.Jun 21 2018, 3:58 PM
TBolliger closed this task as Resolved.
TBolliger updated the task description. (Show Details)

We have T170001: Support CodeMirror syntax highlighting on RTL wikis to tackle RTL support, and T101246: Highlighted wikisyntax while editing articles [AOI] as a parent task. I do not think we need so many tickets to track the exact same work.