T357482 set up integration with the 2017 editor. However, CM6's highlighting has become more expensive in a way that makes the viewport not being clipped really slow.
Currently this is worked around by disabling CodeMirror when a sufficiently long article is loaded. This is not ideal.
The CodeMirror help page says the long-term plan is to adjust the 2017 editor's viewport to be more friendly to CodeMirror.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | Goal | None | T436604 CodeMirror MW 1.47 tracking | ||
| Resolved | MusikAnimal | T432558 The 2017 wikitext editor performs very poorly when CM6 is enabled |
Change #1312541 had a related patch set uploaded (by DLynch; author: DLynch):
[mediawiki/extensions/CodeMirror@master] VisualEditor: add CSS Custom Highlight API mode for source editing
I really think the better route to go is to rework the 2017 editor to not have an infinite viewport. I promised a demo at the Hackathon but never got around to it, but it should be achievable with relative ease (for someone familiar with the VE architecture).
The tricky bit is getting the scrollbar to be for the VE surface itself (not the whole page), but doing so such that things look nice when scrolling to the bottom. What we don't want are two scrollbars – one for the surface and one for the page.
(So, caveat, I'm coming at this as someone who knows plenty about VE, but doesn't know as much about CodeMirror.)
There's a lot of assumptions baked in to VE: that every offset in the model is going to have an associated node that has some rendered DOM, and things like selections assume that if you can select something then you can get a client rect for its DOM nodes. If you click and start dragging and the selection moves far enough that its origin point suddenly gets removed from the page and virtualized, we don't have a way to handle that. Our existing support for content that's in the model but not on the page is visual sections, and there we cope with it by not having any way to affect the unrendered content.
(It's complicated by NWE being VisualEditor, admittedly. Most of these changes would be relatively doable if we could think of them solely as affecting plain text, but because the rendering is VE everything has to get considered in terms of how it'd apply to a mixed visual document.)
The way to avoid the issues is to get rid of the overlay, either by having VE handle its own highlighting (this patch), or by having CodeMirror be the only editor surface and pass changes through to a invisible backing NWE surface and a visible NWE toolbar.
Pros:
Cons:
Pros:
Cons:
We should merge the patch now, test it out via the URL parameter, and probably turn it on for everyone (maybe after implementing line numbers as a native NWE feature) if the performance seems improved on real content with real people.
We can still consider the CodeMirror-as-editor route in the future. This doesn't block us from pursuing it as the editor surface, if we can build the appropriate translation layer.
The CSS custom highlight API is fairly new compared with MW's browser support range (even for Grade A browsers).
The CSS custom highlight API is fairly new compared with MW's browser support range (even for Grade A browsers).
True! That said, per caniuse the only major browser that hasn't supported it for 3+ years is Firefox (which added support about four months ago).
I think a disabled-by-default feature like this which degrades fairly gracefully is a pretty good candidate for using it. I suppose that we actually could write it to fall back to the worse-performing version if the browser is unsupported, though I'm not sure if the maintenance-burden is worth it.
Change #1313243 had a related patch set uploaded (by DLynch; author: DLynch):
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add line numbers to the custom-highlight mode
I still wonder which is harder – re-implementing bits of CodeMirror that we want for VE and trying to keep the duplicated logic and styling in sync with CodeMirror, or just focusing on the getting the 2017 toolbar to play nice with CodeMirror.
Bhsd and I are the opposite, but with our powers combined I think we can get something to work! :)
I agree with your assessment at T432558#12139172 in large part. At least, this is all worth the experimentation, and a good use of AI in my opinion.
- Containing the entire document inside a scrollable container would be easy, but doesn't do anything about the cost because CodeMirror still needs to know the exact height of everything above the top of the container viewport in order to perfectly align the overlay with the VE rendering... so it still has to do the expensive bit. (This is the thing I feel I'm most likely to be wrong about.)
If you give CodeMirror a normal, scrollable textarea or contenteditable (the latter is not documented but it works when the surface property is set, such as with the 2017 editor), it will paint within its viewport and be perfectly performant. You do need to remove the viewState.printing = true, of course. I have tested this many times in my attempts at solving 2017 editor compatibility.
As a quick experiment, try the following CSS in Vector-2022:
.ve-init-mw-desktopArticleTarget-originalContent { .mw-body-content { height: 80vh; overflow: auto; } .ve-ce-surface { height: 100%; } }
With this alone, you can see things are far from workable (huge misalignment on large pages if you hit ⌃+End), but if you for example hold the PgDn key from the top, or click to mid-article, the painting of the colors shows pretty quickly. I don't think CodeMirror is struggling here, rather it's just the dual-surfaces painting things differently that cause UX issues. I suppose that's stating the obvious, hehe, but my point is just to stress that the infinite viewport issue is probably our core issue with respect to performance.
There's a lot of assumptions baked in to VE: that every offset in the model is going to have an associated node that has some rendered DOM, and things like selections assume that if you can select something then you can get a client rect for its DOM nodes.
That explains a lot! Thank you. If it helps, CodeMirror can also give you a client rect given positions or a selection (see Editorview#posAtCoords).
If you click and start dragging and the selection moves far enough that its origin point suddenly gets removed from the page and virtualized, we don't have a way to handle that. Our existing support for content that's in the model but not on the page is visual sections, and there we cope with it by not having any way to affect the unrendered content.
But that's only if VE is responsible for user input, right?
(It's complicated by NWE being VisualEditor, admittedly. Most of these changes would be relatively doable if we could think of them solely as affecting plain text, but because the rendering is VE everything has to get considered in terms of how it'd apply to a mixed visual document.)
The way to avoid the issues is to get rid of the overlay, either by having VE handle its own highlighting (this patch), or by having CodeMirror be the only editor surface and pass changes through to a invisible backing NWE surface and a visible NWE toolbar.
This reminds me of the idea I pitched to @Esanders a few years ago. It seems to me it's the toolbar mostly that matters here. For it to work, it needs to know the context of the cursor placement, selections, and their exact coordinates. Both VE and CodeMirror work off of a transaction-based system. The idea I had – and bear with me as this probably sounds dumb – was to skip the VE surface entirely and just feed the data model from CodeMirror, effectively translating the transactions so that they speak the same language. All the methods the toolbar needs and whatever reactivity that comes with it would need to be reimplemented to some degree (basically we'd make a virtual VE surface), but at the end of the day, the fact is CodeMirror knows where the cursor is, its DOM elements and its coordinates just as well as VE does, they only differ because they're separate surfaces. So it seems it's a matter of just translating things and overriding the VE methods the toolbar needs so that they talk to CodeMirror for positioning tooltips and what not. Does that make any sense? I know it's of course easier said than done.
I just feel if you need CodeMirror to handle display, it should handle input, too. That diversion seems to be the crux of most bug reports.
- CM as the only visible editor
Pros
- All the CM features are there and just work.
Please also keep in mind all the features we'd gain in addition to VE features if CodeMirror were the only visible surface, in particular code folding.
…
Cons:
- We have two documents that need to be kept in sync. Keyboard text edits need to be synced to the NWE model, and toolbar / inspector edits need to be synced back to the CM text. (And any drift away is going to cause data-loss when a change incorrectly applies.)
I think this is a con either way, technically. Both VE and CodeMirror have separate data models (if you will) and separate user-facing surfaces. So the main thing to me is improving performance and UX issues, which seems to mostly fall on there being two surfaces overlaid on top of each other, with one of those surfaces needing to be infinite.
- The toolbar *deeply* assumes that it's working with a VE surface.
Hopefully that part we can change with minimal effort on the VE side, if we're able to iron out the "translation layer" between VE and CodeMirror.
- All the NWE UI also *deeply* assumes it's working with a VE surface. Things like "where should this inspector be positioned?" are based on getting the position of the selection in the VE surface, etc.
This part I feel more confident we can fix, assuming we're content with CodeMirror being the only visible surface.
- Likely locks us out of turning on collaborative editing for source mode for similar "all positions are based on the VE surface" reasons. (Granted, this is unreleased, so it's a constraint on the future rather than breaking a live feature.)
Note there is also https://codemirror.net/examples/collab/
- Basically, I think there's a lot of work to be done making the integration layer.
Absolutely. What we have now seems to work well enough, unless it's a very large page, which is again why from a high-level product perspective, I keep wanting to focus on simply making VE have an finite viewport. Not so simple, though, I know!
What I think we should do
We should merge the patch now, test it out via the URL parameter, and probably turn it on for everyone (maybe after implementing line numbers as a native NWE feature) if the performance seems improved on real content with real people.
I've no issue with giving this a try. Also I'm not in charge 😛. However I am genuinely concerned about product and code drift. CodeMirror has a lot more to come. The relation chain up for review currently re-implements many things from CodeMirror that will be difficult to maintain separately. Please feel free to abstract things out more if that makes the code more DRY and future-proof. I very meticulously denoted methods as @internal and @stable where appropriate (sadly these annotations do not show up on doc.wikimedia.org). You'll have to look at the source, but rule-of-thumb should maybe be that internal methods should not be re-implemented, to save us both money from all the headache medications we'll have to buy further down the road.
We can still consider the CodeMirror-as-editor route in the future. This doesn't block us from pursuing it as the editor surface, if we can build the appropriate translation layer.
Sure thing! This task and everything CodeMirror is and ever will be is because of the grace of the Editing team. Needless to say, we trust you :)
I don't think CodeMirror is struggling here, rather it's just the dual-surfaces painting things differently that cause UX issues. I suppose that's stating the obvious, hehe, but my point is just to stress that the infinite viewport issue is probably our core issue with respect to performance.
Yeah, sorry, I should have said that I don't think this is a fundamental problem with CodeMirror, more with the situation we're fitting it into. I also agree that the core issue is that there's multiple surfaces we have to keep in alignment.
Near as I can tell, the difficult part is that anything that involves removing viewState.printing = true is going to have to either remove one of the surfaces or solve the alignment problem.
CodeMirror's viewport-calculation seems to already be limited to the window's viewport, so without printing sticking VE into a container doesn't actually change the area to highlight. It's a stepping stone towards a real fix, in the sense that any virtualizing-the-VE-surface is going to do something close to that... it's just that the subsequent "and then fix the alignment problem without printing" step is the hard one. Or rather, if we could cheaply fix the alignment issue, we could disable printing and whether we had VE in a container or not would just be an implementation-detail of whatever fix we had chosen.
(As far as I can see, the reason paging-down gets a different-and-better alignment versus jumping to the end is that CM's measureVisibleLineHeights caches real results after they're visible in-viewport, so if you've actually-seen the entire page it should be correct... whereas hitting end gets you the maybe-extremely-wrong heights from heightForLine.)
If it helps, CodeMirror can also give you a client rect given positions or a selection (see Editorview#posAtCoords).
[...]
But that's only if VE is responsible for user input, right?
[...]
So it seems it's a matter of just translating things and overriding the VE methods the toolbar needs so that they talk to CodeMirror for positioning tooltips and what not. Does that make any sense? I know it's of course easier said than done.
My main worry with this approach is that it feels both complex and fragile. In concept it could absolutely work, but the translation layer would be a pain to write to map everything correctly, and then would be very prone to subtle breakages from either direction.
Essentially, ve.ce.Surface has a lot of behavior in it tied to observing inputs, and reducing it down to applying transactions from CM to the VE data model bypasses all that. I'm sure CodeMirror has its own equivalents of things like that, but it's presumably not a 1:1 match, and that starts to bring up expectation mismatches.
Quick example of a fun thing that'd be a nuisance to do in this model: NWE notices when you paste rich content and offers to convert it to wikitext for you. It can only do this because you're pasting it into a surface it controls, where the specific source-mode MWWikitextClipboardHandler can simultaneously plaintextify it for the paste, notice that it was rich text, and show a context item which offers to make the conversion. (This is also an example of existing source-specific behavior -- none of it's just inherited from visual.)
Could we tell VE about the paste? Sure, though ce.Surface has a lot of heuristics built up to cope with IMEs and suchlike doing pastes-that-don't-tell-you-they're-pastes... but it's just one of a bunch of relatively-bespoke things we'd need to implement as one-offs.
(That said, an advantage we have is that NWE is already doing fairly limited things compared to VE. E.g. It's okay that annotating content with ImportedDataAnnotation wouldn't happen, because NWE never does anything with that anyway, for now. It'd be adding a bit of a blocker against future EditCheck implementation, admittedly.)
I just feel if you need CodeMirror to handle display, it should handle input, too. That diversion seems to be the crux of most bug reports.
Totally agree, thus this patchset. Which admittedly is approaching it from the opposite direction of going "...but what if CodeMirror didn't handle display either?"
CM as the only visible editor
Pros:
- All the CM features are there and just work.
Please also keep in mind all the features we'd gain in addition to VE features if CodeMirror were the only visible surface, in particular code folding.
The current things on that list that aren't implemented in NWE or these patches:
I'd say the ones that would either remain not-available or would need a serious reimplementation effort are folding, linting, and multiple selections.
Cons:
- We have two documents that need to be kept in sync.
I think this is a con either way, technically.
I think not in a way that matters so much for the VE-in-charge approach. In the patchset there's an EditorState that holds a Text, and whenever the VE document changes it updates that so that we can get the tokenized ranges. However, because CodeMirror never takes an action on it, there's never any need to sync back to VE. We don't need to synchronize the selection, even, because the bracket-matching is done as a separate system. (The CM document is sort of an implementation-detail of us using it as a tokenizer, rather than a document.)
Granted, if something goes wrong and the CM tokenizer is no longer acting on the right text, there'll still be an effective sync issue where the tokenized ranges have drifted away from the correct output. This is notably better than the current CM integration, though, where the same drift would get the text-renderings overlaid incorrectly so it rapidly becomes unreadable.
- The toolbar *deeply* assumes that it's working with a VE surface.
Hopefully that part we can change with minimal effort on the VE side, if we're able to iron out the "translation layer" between VE and CodeMirror.
- All the NWE UI also *deeply* assumes it's working with a VE surface. Things like "where should this inspector be positioned?" are based on getting the position of the selection in the VE surface, etc.
This part I feel more confident we can fix, assuming we're content with CodeMirror being the only visible surface.
So, here's a best-case summary of showing a link inspector at the cursor:
That's actually all pretty doable as a simple translation. Some state management, some passing through of CM's selection-positions, some faking of what the surface thinks is "focused", etc.
The trickier stuff seems to happen when we want to deal with parts of the document that aren't visible. A good example would be getting VE's find/replace toolbar working while driving CM's surface. Find/replace in VE works by finding a bunch of text ranges and then drawing highlights over them, then when you click next/prev it calls its scrollSelectionIntoView... which works by finding the position of an off-screen element and scrolling to it.
The problem is that CM's docs say for domAtPos:
Note that for positions that aren't currently in visibleRanges, the resulting DOM position isn't necessarily meaningful (it may just point before or after a placeholder element).
I.e. if we start asking it for the locations of things off-viewport, it's going to start making up answers (or giving approximations, at least). Not terrible for the highlight-drawing, because we wouldn't see those anyway, but a real problem for pressing "next" in the find/replace toolbar.
(CM's own find/replace seems to dodge this by being implemented differently: the highlights are mapped onto view.visibleRanges and get recomputed on viewportChanged. It never thinks about rects. This is very fair in a text-only editor, and is an example of how the more general visual nature of VE makes it harder to optimize when we know it's all plain-text.)
We could absolutely know to do things like substitute how find/replace expects to work, or swap in CM's find/replace implementation for this specific tool. But it's a pattern of needing to hunt for painful interactions like this that seems challenging...
Plus, some of them can't really be pulled out like find/replace can. To go back to that link example: select some text, scroll it off the viewport, then click the toolbar link icon. In NWE you'll get scrolled back to it for the popup to show. Because CM doesn't know positions until it has actually rendered the region, it can't answer "where is that?" up front at all; we'd have to hand the scroll itself off to EditorView.scrollIntoView, wait for it to settle, and only then ask for the rect to position the UI. (Once the line is rendered, CM's coordinates are exact; it's the ordering that's the problem, not the accuracy.)
Again, a translation layer can probably cope via passing all scroll-to-a-place off to EditorView.scrollIntoView, waiting for it to be done so coordinates are known, then telling VE to do the UI positioning. (Does imply some extensive promise / await refactoring for context-positioning code in VE, though.)
Note there is also https://codemirror.net/examples/collab/
True, though I'm not sure how practical it would be to bolt a different collab system into the existing VE collab UX. (The VE collab UX does useful things like coordinate who's allowed to publish, getting attribution agreements, etc.)
However I am genuinely concerned about product and code drift. CodeMirror has a lot more to come. The relation chain up for review currently re-implements many things from CodeMirror that will be difficult to maintain separately.
My take on this is that the current patches aren't too bad in terms of reimplementation. They absolutely reuse a bunch of things, but so long as CodeMirror's tokenizer / bracket-matching interfaces remain stable, they'll now be hooked up to a similarly stable part of VE's rendering pipeline that we'd expect to keep maintained.
The advantage in terms of maintainability is that everything we can leverage as part of the core VE source editor becomes something that the editing team will keep working because it's right in front of us as we change things... as opposed to the CodeMirror extension having to scramble around because we changed something about the VE rendering and didn't know to test some part of how it interacts with the CM overlay.
(Sorry, this reply got a bit away from me in length and in the amount of diving-into-CodeMirror involved. 😅)
Change #1316993 had a related patch set uploaded (by DLynch; author: DLynch):
[mediawiki/extensions/CodeMirror@master] VisualEditor: Size headings in the custom-highlight mode
Change #1312581 had a related patch set uploaded (by DLynch; author: DLynch):
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add bracket matching to the custom-highlight mode
Change #1312541 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] VisualEditor: add CSS Custom Highlight API mode for source editing
Change #1312581 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add bracket matching to the custom-highlight mode
Change #1318737 had a related patch set uploaded (by DLynch; author: DLynch):
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add active line and trailing whitespace to custom-highlight
Change #1318738 had a related patch set uploaded (by DLynch; author: DLynch):
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add whitespace marks to the custom-highlight mode
Test wiki created on Patch demo by DLynch (WMF) using patch(es) linked to this task:
https://6eadbd6089.catalyst.wmcloud.org/w/
Change #1316993 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] VisualEditor: Size headings in the custom-highlight mode
Change #1318737 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add active line and trailing whitespace to custom-highlight
Change #1318738 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add whitespace marks to the custom-highlight mode
Change #1313243 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] VisualEditor: Add line numbers to the custom-highlight mode
Change #1322175 had a related patch set uploaded (by DLynch; author: DLynch):
[operations/mediawiki-config@master] CodeMirror: turn on the new 2017 editor integration
Test wiki on Patch demo by DLynch (WMF) using patch(es) linked to this task was deleted:
Change #1338308 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):
[operations/mediawiki-config@master] [CodeMirror] enable for new users (enwiki), new VE integration (global)
Change #1322175 merged by jenkins-bot:
[operations/mediawiki-config@master] CodeMirror: turn on the new 2017 editor integration
Mentioned in SAL (#wikimedia-operations) [2026-09-10T22:01:00Z] <musikanimal@deploy1003> Started scap sync-world: Backport for [[gerrit:1322175|CodeMirror: turn on the new 2017 editor integration (T432558)]], [[gerrit:1338308|[CodeMirror] enable for new and logged-out users by default on enwiki (T288161)]]
Mentioned in SAL (#wikimedia-operations) [2026-09-10T22:05:46Z] <musikanimal@deploy1003> kemayo, musikanimal: Backport for [[gerrit:1322175|CodeMirror: turn on the new 2017 editor integration (T432558)]], [[gerrit:1338308|[CodeMirror] enable for new and logged-out users by default on enwiki (T288161)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.
Mentioned in SAL (#wikimedia-operations) [2026-09-10T22:12:00Z] <musikanimal@deploy1003> Finished scap sync-world: Backport for [[gerrit:1322175|CodeMirror: turn on the new 2017 editor integration (T432558)]], [[gerrit:1338308|[CodeMirror] enable for new and logged-out users by default on enwiki (T288161)]] (duration: 10m 59s)
Change #1341273 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):
[operations/mediawiki-config@master] [CodeMirror] enable for new users (enwiki), new VE integration (global)
Change #1341273 merged by jenkins-bot:
[operations/mediawiki-config@master] [CodeMirror] enable for new users (enwiki), new VE integration (global)
Mentioned in SAL (#wikimedia-operations) [2026-09-14T19:04:34Z] <musikanimal@deploy1003> Started scap sync-world: Backport for [[gerrit:1341273|[CodeMirror] enable for new users (enwiki), new VE integration (global) (T288161 T432558)]]
Mentioned in SAL (#wikimedia-operations) [2026-09-14T19:22:15Z] <musikanimal@deploy1003> musikanimal: Backport for [[gerrit:1341273|[CodeMirror] enable for new users (enwiki), new VE integration (global) (T288161 T432558)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.
Mentioned in SAL (#wikimedia-operations) [2026-09-14T19:38:25Z] <musikanimal@deploy1003> Finished scap sync-world: Backport for [[gerrit:1341273|[CodeMirror] enable for new users (enwiki), new VE integration (global) (T288161 T432558)]] (duration: 33m 51s)
The new Highlight API version is now enabled everywhere at WMF by default. We should let it simmer for a while before removing the feature flag and the older CSS hack implementation, but I think it's safe to consider this task resolved. Follow-ups, as are sure to come, should be filed as new tasks as necessary.
Ten thousand thanks to @DLynch for his hard work on this!