Page MenuHomePhabricator

Get basic CodeMirror 6 setup working with ResourceLoader
Closed, ResolvedPublic8 Estimated Story Points

Description

This is the first step of T259059: Upgrade to CodeMirror 6. This will only put us in a basic working state with CodeMirror 6, with everything behind a temporary feature flag. The actual syntax highlighting and other features are to follow.

Acceptance criteria

  • Using the basic CodeMirror package, identify which modules under the @codemirror namespace we need.
  • Create new ext.CodeMirror.v6.WikiEditor module that is loaded when $wgCodeMirrorV6 is set or if the cm6enable=1 URL query parameter is set.
  • Visually, enabling CodeMirror should give you a monospaced text editor without any syntax highlighting.
    • Relevant features of WikiEditor should still work (no exhaustive testing necessary), and you should still be able to save the page, etc.
    • Identify any other integrations that are apparently now broken, and document them (we're not going to enable $wgCodeMirrorV6 anytime soon so some breakage is expected)

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. Β· View Herald Transcript

Copy the raw source of those files into the resources/lib/ directory of the CodeMirror MediaWiki extension.

This should probably be done with a manageForeignResources.php maintenance script.

MusikAnimal set the point value for this task to 8.Sep 21 2022, 7:05 PM

Change 961145 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/CodeMirror@master] [WIP] CodeMirror 6 upgrade

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

I still need to write some tests but I think this is ready for initial review.

I want to fix the Realtime Preview integration, so back to in-dev for now.

Change 961145 merged by jenkins-bot:

[mediawiki/extensions/CodeMirror@master] CodeMirror6: add new modules, feature flag, and URL query parameter

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

TheresNoTime added a subscriber: TheresNoTime.

Testing on the beta cluster with cm6enable=1 (and then toggling syntax highlighting) gives the following error:

Uncaught TypeError: t.view.scrollIntoView is not a function
    at jQuery.fn.init.scrollToCaretPosition [...]

Disabling realtime preview and retrying does not show this error, but instead errors with

Uncaught TypeError: Cannot read properties of undefined (reading 'off')
    at bindTextareaListener [...]

when toggling syntax highlighting off.

Testing on the beta cluster with cm6enable=1 (and then toggling syntax highlighting) gives the following error:

Uncaught TypeError: t.view.scrollIntoView is not a function
    at jQuery.fn.init.scrollToCaretPosition [...]

Disabling realtime preview and retrying does not show this error, but instead errors with

Uncaught TypeError: Cannot read properties of undefined (reading 'off')
    at bindTextareaListener [...]

when toggling syntax highlighting off.

I can't repro the first issue, but the second is a broken integration with Dismambiguator's notification feature. Since CodeMirror 6 is not backwards compatible, any integration is going to break until fixed.

EDIT: I can repro the RTP conflict and am working on a fix.

Change 964975 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/CodeMirror@master] CodeMirrorWikiEditor: add missing hook and fix reference to textarea

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

…
Disabling realtime preview and retrying does not show this error, but instead errors with

Uncaught TypeError: Cannot read properties of undefined (reading 'off')
    at bindTextareaListener [...]

when toggling syntax highlighting off.

… the second is a broken integration with Dismambiguator's notification feature. Since CodeMirror 6 is not backwards compatible, any integration is going to break until fixed.

Lies… we weren't even firing the hook in the new code! Also, while the CodeMirror API itself is not backwards compatible, any use of jQuery.textSelection() against the .cm-editor element should work fine, which is the case with Disambiguator.

Change 964998 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/CodeMirror@master] CodeMirror: fix scrollToCaretPosition $.textSelection implementation

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

Change 964975 merged by jenkins-bot:

[mediawiki/extensions/CodeMirror@master] CodeMirrorWikiEditor: add missing hook and fix reference to textarea

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

Change 964998 merged by jenkins-bot:

[mediawiki/extensions/CodeMirror@master] CodeMirror: fix scrollToCaretPosition $.textSelection implementation

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

(Something is up with my local dev vs. beta cluster β€” though even then these bugs seem intermittent..)

As currently live on beta, I seem to be able to occasionally trigger a bug where enabling/disabling/enabling RTP (with cm6enable=1 set) causes:

VM1393:8 Uncaught DOMException: Failed to execute 'insertBefore' on 'Node': The new child element contains the parent.
    at HTMLDivElement.<anonymous> [...]
Uncaught TypeError: Cannot read properties of undefined (reading 'done')
    at RealtimePreview.doRealtimePreview [...]

and the resultant UI:

Screenshot 2023-10-11 at 10.22.07.png (571Γ—940 px, 127 KB)

Change 966650 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/CodeMirror@master] CodeMirrorWikiEditor: sync with textarea when using CodeMirror toggle

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

Change 967539 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/CodeMirror@master] CodeMirrorWikiEditor: scroll selection into view on inital load

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

Change 966650 merged by jenkins-bot:

[mediawiki/extensions/CodeMirror@master] CodeMirrorWikiEditor: sync with textarea when using CodeMirror toggle

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

Change 967539 merged by jenkins-bot:

[mediawiki/extensions/CodeMirror@master] CodeMirrorWikiEditor: scroll selection into view on inital load

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

I'm going to mark this as resolved. Although it can be tested early, the QA effort should probably be saved closer to when we have a MVP, in particular T348019.