[go: up one dir, main page]

Page MenuHomePhabricator

Support JavaScript hook events for CodeMirror
Open, LowPublic

Description

When CodeMirror element structure changes, JS world should be notified.

  1. After initial page DOM loading, JS generation of hilite layer starts. I want to know when CodeMirror element creation finished.
  2. When source text changes significantly, I want to get notified. CodeMirror elements may have been created, or entire elements could vanish due to deletions.

Both situations may be reflected by only one event; point 1 is a special case of event 2.

  • Single characters amid element content are less important; I could listen change events and follow each keystroke anyway.

In MediaWiki context mw.hook() would be preferred and robust mechanism.

  • If non-MediaWiki environment needs to be supported, user defined JS Event() might be triggered.

Use cases:

  • One existing feature is drawing attention on errors, here as soon as .cm-error shows up, avoiding saving and leaving before remedied.
  • Another interesting issue is the occurrence of certain templates from a list of observed template names, flipping through .cm-mw-template-name and even more if connected with some template parameters which might be deprecated and should be replaced, or evaluating parameter values and warn or fix or format. Grabbing CodeMirror elements might be a new approach for next generation tools.

The other way around: CodeMirror might listen to mw.hook() and gets nudged to re-start for entire textarea content.

  • Extension talk:CodeMirror #How to send a command to CodeMirror from JS? is complaining.
  • Apparently current implementaton is listening to keystroke or mouse events only.
  • By JavaScript activities the text might be changed, too.
    • Each other single change event might be caught, too. However, it could be more efficient that JavaScript tools fire a hook event anyway, as soon as changes are completed. If CodeMirror is not active this is rather cheap. If CodeMirror is active rebuilding could be triggered after all incremental changes have been performed.

Event Timeline

My ActualLivePreview script also needs a way to hook into the change event of the textarea, and it's missing this now.

Change 375613 had a related patch set uploaded (by Pastakhov; owner: Pastakhov):
[mediawiki/extensions/CodeMirror@master] Add CodeMirror enabled/disabled JS hooks

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

As a variant the patch above calls the CodeMirror.enabled and CodeMirror.disabled hooks.
Catching ones gets you full access to the CodeMirror features.
For example, alert message when the text is changed:

mediaWiki.hook( 'CodeMirror.enabled' ).add( 
    function ( cm ) { 
        cm.on( 'change', function() { 
            alert ( 'text changed' ); 
        } );
    } 
);

It used event change of CodeMirror.

I'm not sure this is the best approach as it creates two APIs for users who want to listen to text change events. Users shouldn't have to care which editor is being used, they should receive the same event for CodeMirror changes and textarea changes.

I am listening to textarea changes for many years already, since these are covered by low-level DOM events. I did not ask for this.

What I want to know is a structural change in syntax; new elements created or vanishing.

I do not want to restart the entire analysis for all classes after every single keystroke; I want to consider new or removed elements by their classes.

BTW the application object passed by the patch is exactly what will cover all the needs; thanks a lot.

I don't think relying on CM for wikitext parsing via the CM DOM is a good idea - it's a very indirect and fragile way of looking for wikitext structure. The CodeMirror highlighting classes should not be considered a public API, or a definitive source of model information, it's for presentation only.

Please see the use cases mentioned above:

  • One existing feature is drawing attention on errors, here as soon as .cm-error shows up, avoiding saving and leaving before remedied.

Please tell me a performant way to check for this error class, beside your suggestion to re-examine the entire DOM after every keystroke and every mouse click.

With CodeMirror, all my listening for $( '#wpTextbox1' ).blur() is worthless. What can be done instead of listening to two element's events?

Is here way to edit CodeMirror's textarea and check is CodeMirror enabled?

edit: I mean the hooks ended up not being added so is here any alternative to access it?

edit 2: Never mind figured out myself. But here is how it's done.

It seems like any customisation or extendability of CodeMirror by JavaScript developers was completely left out of development by Community Tech team. I’ve first noticed it in T214989 (not exactly first-hand), now I am seeing more issues trying to develop a script with keyboard shortcuts for WikiEditor (link for curious), and this is the most serious issue out of all. It’s really strange to see an important end-user tool to lack all of this stuff.

It seems like any customisation or extendability of CodeMirror by JavaScript developers was completely left out of development by Community Tech team.

Because It wasn't the primary target of the couple of weeks of development spent on the project: https://meta.wikimedia.org/wiki/Community_Tech/Wikitext_editor_syntax_highlighting

Because It wasn't the primary target of the couple of weeks of development spent on the project: https://meta.wikimedia.org/wiki/Community_Tech/Wikitext_editor_syntax_highlighting

You don’t need to explain it to me. At the same time, though, TemplateWizard was done by the same team and it has far more possibilities of third-party usage, even though it also wasn’t a target of development. So it’s just an unfortunate oversight.

Removing task assignee due to inactivity, as this open task has been assigned for more than two years (see emails sent to assignee on May26 and Jun17, and T270544). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be very welcome!

(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)

MusikAnimal subscribed.

I don't think relying on CM for wikitext parsing via the CM DOM is a good idea - it's a very indirect and fragile way of looking for wikitext structure. The CodeMirror highlighting classes should not be considered a public API, or a definitive source of model information, it's for presentation only.

Agreed with this statement, but I see no harm in providing a "change" event in CodeMirror.

What I propose is to emit a "change" event on the textarea whenever content in CodeMirror changes. This is akin to T197632. What you do with the CodeMirror DOM (.cm-content for CodeMirror 6) is up to you and at your own risk. Does that sound okay?

If you want access to CodeMirror internals, that's a different ask. We could introduce a new hook for document changes, passing in the ViewUpdate object along with the CodeMirror instance (this wouldn't make sense to emit with a "change" event on the textarea). I'm a bit hesitant to do this however given the other hooks don't provide access to the CM instance, but I suppose they could… So long as developers are mindful of the frontend stable interface policy (use only methods marked as @stable), I don't see an issue with exposing our interfaces – CodeMirror and CodeMirrorWikiEditor, depending on if WikiEditor is being used. These would give you access to the EditorView.

I'll also note the undocumented, but working hack to get a CodeMirror 6 instance:

const view = document.querySelector( '.cm-content' ).cmView.view;

This can be used but is not something I would consider stable or future-proof.


Note that CodeMirror 6 is coming very soon (T259059), so whatever we do for this task will only be for v6 and not the legacy version currently deployed on most wikis.

@MusikAnimal Note that JS/jQuery's change event, just like input event, is not emitted on any change of the input's content, resulting from both direct user input and programmatic changes, but only on direct user input, and change, as opposed to input, is only fired after the changed input is unfocused. Compare that to OOUI's change event on inputs that covers all changes (and is much more useful in that respect).

So, if an external script just listens to input/change events on the textarea, emitting change on any change may lead to inconsistencies. On the other hand, given the deficiency of the native events on the part of reacting to programmatic changes, I don't see a reason to treat them as a standard to be adhered to. So, when it comes to @Esanders's point:

I'm not sure this is the best approach as it creates two APIs for users who want to listen to text change events. Users shouldn't have to care which editor is being used, they should receive the same event for CodeMirror changes and textarea changes.

...then maybe introducing another, text selection API-specific event that acts like OOUI's change event could be a solution. Or, if that's needed and possible, native-JS input and change events could be imitated and a CodeMirror instance provided to capture programmatic changes. (But does anybody really need native change events on textareas?)

Meanwhile, here's a list of jQuery events that Convenient-Discussions listens to on its comment input: dragover dragleave dragend drop blur paste. Probably, apart from blur which is already supported, they can be listened to on some wrapper div instead. But if the goal is to provide a uniform interface for CodeMirror and plain textareas/WikiEditor, then maybe some gate should be established from arbitrary events on CodeMirror DOM to native JS events on textarea.

Note that JS/jQuery's change event, just like input event, is not emitted on any change of the input's content, resulting from both direct user input and programmatic changes, but only on direct user input, and change, as opposed to input, is only fired after the changed input is unfocused. …

So, if an external script just listens to input/change events on the textarea, emitting change may lead to inconsistencies for clients.

On the other hand, given the deficiency of the native events on the part of reacting to programmatic changes, I don't see a reason to treat them as a standard to be adhered to.

Thanks for pointing this out. In CodeMirror 6, we monitor changes using EditorView.updateListener(). If one of the hooks gave you the CM instance, you could register your own listener that way. I think ideally though clients wouldn't be required to do more than listen to the textarea and not care if CodeMirror is involved or not, since CM can be activated programatically and we shouldn't have to force clients to use hooks to know what element to listen to. In that regard, I suppose using the input event then is better, but it still wouldn't be one-to-one identical to the native event. We'd check if the ViewUpdate has the docChanged property set. This nicely sort of groups changes to basically what would be undone with an undo operation. I.e. not every keystroke, but every sequence of changes in a short time (I'm not exactly sure as to the logic, to be honest).

While we want to bubble the events to the textarea, we could also do the same for the .cm-content element (the contentEditable), as such events may be expected to work there, too.

How does that sound?

@MusikAnimal I never thought of differentiating between direct user input and programmatic changes as of a requirement of some client. So if there is really no such need, probably emitting input would be OK. I also haven't found any requests on Phabricator to emit events on programmatic changes of the native textarea. So maybe there is no need to devise a solution for a problem that has not yet surfaced.

I'll also note the undocumented, but working hack to get a CodeMirror 6 instance:

const view = document.querySelector( '.cm-content' ).cmView.view;

This can be used but is not something I would consider stable or future-proof.

Do I understand it right that event handlers can be added only before the initialization stage, by providing an extension like here? So an already initialized instance would be of no use if we want to add handlers. This would be bad news for the "load ext.CodeMirror.v6.WikiEditor and forget" method we came up with in T214989.

There is the module jquery.textSelection at rMW /src/jquery/jquery.textSelection.js

  • This is wrapping access to selected text or cursor position independent of browser methods, and does also permit text changes at cursor position or replacing marked text range.

This does currently not support CodeMirror but native text <input> and <textarea>.

  • It should cover CodeMirror and DiscussionTools transparently as well.

In a similar way I would like to get a wrapper around CodeMirror and DiscussionTools, which shall allow to deal with text fields no matter which mode the user currently toggled:

  • Retrieve textarea#wpTextbox1.val() as entire text
  • Assign textarea#wpTextbox1.val(s) to the entire story
  • If currently in CodeMirror or DiscussionTools mode, then retrieve via the corresponding API; if native then jQuery .val() function.
  • There is a second text field on Special:Upload.

In addition I want to explore whether the user input did change CodeMirror text and possible errors have been marked.

  • It is sufficient to know that the text has changed, no matter whether by programmatic assignment of entire or partial text or mouse event or keyboard event. When it changed I can look for arriving or vanished error class elements.
  • I need to know whether CodeMirror was activated or left. While no CodeMirror is active I do not need to look for error class elements. Events or mw.hook() shall be fired when CodeMirror toggled.

Do I understand it right that event handlers can be added only before the initialization stage, by providing an extension like here? So an already initialized instance would be of no use if we want to add handlers.

Dynamic configuration is a thing, but it might be tricky (docs) and would likely require some changes in the CodeMirror extension. I was planning to look more into this for T359498: Add preferences panel to CodeMirror 6, so I don't fully understand the challenges involved just yet. T359498 would be be to dynamically toggle specific features, but I think using the upstream CodeMirror API you should still be able to do what you need. I can look into making this easier with a wrapper API in the extension, avoiding any need to have access to CodeMirror internals (since the ext.CodeMirror.v6.lib module is transpiled code with no stable entrypoint).

This would be bad news for the "load ext.CodeMirror.v6.WikiEditor and forget" method we came up with in T214989.

I think you can still do it by adding CodeMirror to your existing WikEditor instance (in the case of CD). I tested it locally. Something akin to:

mw.loader.using( [ 'ext.CodeMirror.v6', 'ext.CodeMirror.v6.mode.mediawiki' ] ).then( ( require ) => {
	const CodeMirror = require( 'ext.CodeMirror.v6' );
	const mediawikiLang = require( 'ext.CodeMirror.v6.mode.mediawiki' );
	const cm = new CodeMirror( $( 'textarea' ) );
	cm.initialize( [ cm.defaultExtensions, mediawikiLang() ] );
} );

At cursory glance, the only thing missing was the toggle button, which you could add manually for now.

There is the module jquery.textSelection at rMW /src/jquery/jquery.textSelection.js

  • This is wrapping access to selected text or cursor position independent of browser methods, and does also permit text changes at cursor position or replacing marked text range.

This does currently not support CodeMirror but native text <input> and <textarea>.

  • It should cover CodeMirror and DiscussionTools transparently as well.

I think the idea is to do the reverse – editors are expected to register their own jQuery.textSelection implementation, so that we always have a stable API. CodeMirror does this. I'm not sure about DiscussionTools.

In a similar way I would like to get a wrapper around CodeMirror and DiscussionTools, which shall allow to deal with text fields no matter which mode the user currently toggled:

Unless I'm misunderstanding, that's what we have. You should only ever need to work with the textarea as far as jQuery.textSelection goes, but you can do it on the contenteditable .cm-content too.

In addition I want to explore whether the user input did change CodeMirror text and possible errors have been marked.

  • It is sufficient to know that the text has changed, no matter whether by programmatic assignment of entire or partial text or mouse event or keyboard event. When it changed I can look for arriving or vanished error class elements.

I'm fine with this and is essentially what I proposed in the above comments. We can provide you with a ViewUpdate object which I think will give you everything you need. The question I guess is whether you'd rather get this via a hook or provide your own Extension when CodeMirror is initialized. Currently, the latter can be tricky if you want CodeMirror + WikiEditor.

I'm not sure of your use case, but in case it matters -- I was hoping to eventually denote errors more clearly (eventually behind a preference) as well as add linting, etc. If you wanted to do something similar, consider helping with this effort in the extension! :)

  • I need to know whether CodeMirror was activated or left. While no CodeMirror is active I do not need to look for error class elements. Events or mw.hook() shall be fired when CodeMirror toggled.

See https://www.mediawiki.org/wiki/Extension:CodeMirror/6#Using_hooks. CodeMirror 5 has only the ext.CodeMirror.switch hook. You could also check the usecodemirror preference (at least for WikiEditor), or I guess look for the presence of .cm-editor or .cm-content in the DOM.

Another idea that just occurred to me: We could provide the CodeMirrorWikiEditor class as its own resource module that doesn't require ext.wikiEditor, instead allowing you do that on your own by whatever means. Seems like that might make things easier?

I think you can still do it by adding CodeMirror to your existing WikEditor instance (in the case of CD). I tested it locally. Something akin to:

mw.loader.using( [ 'ext.CodeMirror.v6', 'ext.CodeMirror.v6.mode.mediawiki' ] ).then( ( require ) => {
	const CodeMirror = require( 'ext.CodeMirror.v6' );
	const mediawikiLang = require( 'ext.CodeMirror.v6.mode.mediawiki' );
	const cm = new CodeMirror( $( 'textarea' ) );
	cm.initialize( [ cm.defaultExtensions, mediawikiLang() ] );
} );

At cursory glance, the only thing missing was the toggle button, which you could add manually for now.

Yeah, already figured that out, although I think you would also need to do const lib = require( 'ext.CodeMirror.v6.lib' ); to get access to EditorView.updateListener and do something like this to emit input events on Special:BlankPage:

const $textarea = $( '<textarea>' );
$( '.mw-body-content' ).append( $textarea );
const require = await mw.loader.using( [ 'ext.CodeMirror.v6', 'ext.CodeMirror.v6.mode.mediawiki' ] );
const CodeMirror = require( 'ext.CodeMirror.v6' );
const mediawikiLang = require( 'ext.CodeMirror.v6.mode.mediawiki' );
const lib = require( 'ext.CodeMirror.v6.lib' );
const myExtension = lib.EditorView.updateListener.of( ( update ) => {
  if ( update.docChanged ) {
    $textarea.trigger( 'input' );
  }
} );
const cm = new CodeMirror( $textarea );
cm.initialize( [ cm.defaultExtensions, mediawikiLang(), myExtension ] );

But if feels like CodeMirrorWikiEditor does a considerable amount of work in addition to adding the button to the toolbar, and this amount may grow in the future as the components become more integrated. So I think the preferred way, which allows to avoid duplication of code and effort, would still be to provide CodeMirrorWikiEditor. If this is done together with making WikiEditor officially support arbitrary textareas, like I advocated in T214989#9709038 and T362356, that would be the best option I think. If you prefer an alternative like

Another idea that just occurred to me: We could provide the CodeMirrorWikiEditor class as its own resource module that doesn't require ext.wikiEditor, instead allowing you do that on your own by whatever means. Seems like that might make things easier?

that would be good also. Dynamic configuration? Would also work if we manage to wrap our heads around it. Do as you deem appropriate and realistic, I guess.

Another idea that just occurred to me: We could provide the CodeMirrorWikiEditor class as its own resource module that doesn't require ext.wikiEditor, instead allowing you do that on your own by whatever means. Seems like that might make things easier?

(But I didn't quite understand the difference between this and how you implemented this in 1014134 to be used like in T214989#9702978. Which exact part would be delegated to the client?)

There is the module jquery.textSelection at rMW /src/jquery/jquery.textSelection.js

  • This is wrapping access to selected text or cursor position independent of browser methods, and does also permit text changes at cursor position or replacing marked text range.

This does currently not support CodeMirror but native text <input> and <textarea>.

  • It should cover CodeMirror and DiscussionTools transparently as well.

I think the idea is to do the reverse – editors are expected to ...

I do want to use jquery.textSelection in my application, and I do not want to care whether CodeMirror (or DiscussionTools) are covering the native textarea.

I do not know whether the user has toggled in the page two seconds before to CodeMirror, and will toggle to native ten seconds later in the same page.

I want to retrieve from jquery.textSelection where cursor or selection range are, and I do not want to know whether currently CodeMirror is active or not. I want to fire: Insert XYZ at cursor position bur I do not want to care whether this is in CodeMirror nor native nor DiscussionTools mode.

I want to retrieve the entire source text, and I do not want to care whether CodeMirror has been toggled on or not. And I want to assign the entire source text as page content, and it is business of the wrapper to detect whether CodeMirror is on or not.

There is the module jquery.textSelection at rMW /src/jquery/jquery.textSelection.js
...
This does currently not support CodeMirror but native text <input> and <textarea>.

It does.

I want to retrieve from jquery.textSelection where cursor or selection range are, and I do not want to know whether currently CodeMirror is active or not. I want to fire: Insert XYZ at cursor position bur I do not want to care whether this is in CodeMirror nor native nor DiscussionTools mode.

I want to retrieve the entire source text, and I do not want to care whether CodeMirror has been toggled on or not. And I want to assign the entire source text as page content, and it is business of the wrapper to detect whether CodeMirror is on or not.

This works with CodeMirror just as it does without.

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

[mediawiki/extensions/CodeMirror@master] CM6: Rework ResourceLoader modules

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

I think you would also need to do const lib = require( 'ext.CodeMirror.v6.lib' ); to get access to EditorView.updateListener and do something like this to emit input events on Special:BlankPage:


const lib = require( 'ext.CodeMirror.v6.lib' );

lib.EditorView.updateListener

Wow! Thanks for pointing this out. For some reason I had it in my mind that between the tree shaking and build step, the transpiled code wouldn't maintain the original symbol names. Either way it would be something we can control, so I'm happy to advertise using it in this way. I've documented some examples (with code courtesy of you!) at https://www.mediawiki.org/wiki/Extension:CodeMirror/6#Extending_CodeMirror . Feel free to edit of course.

But if feels like CodeMirrorWikiEditor does a considerable amount of work in addition to adding the button to the toolbar, and this amount may grow in the future as the components become more integrated. So I think the preferred way, which allows to avoid duplication of code and effort, would still be to provide CodeMirrorWikiEditor. If this is done together with making WikiEditor officially support arbitrary textareas, like I advocated in T214989#9709038 and T362356, that would be the best option I think. If you prefer an alternative…

+1 to improving WikiEditor, but as for this task, I'm proposing the following for full control of CM and its internals:

const require = await mw.loader.using( [ 'ext.CodeMirror.v6.WikiEditor', 'ext.CodeMirror.v6.mode.mediawiki' ] );
const CodeMirrorWikiEditor = require( 'ext.CodeMirror.v6.WikiEditor' );
const mediawikiLang = require( 'ext.CodeMirror.v6.mode.mediawiki' );
const lib = require( 'ext.CodeMirror.v6.lib' );
const myExtension = lib.EditorView.updateListener.of( ( update ) => {
	if ( update.docChanged ) {
		// do something
	}
} );
const cm = new CodeMirror( $( 'textarea' ) );
cm.initialize( [ cmWe.defaultExtensions, mediawikiLang(), myExtension ] );

I've added this with r1019841.

Another idea that just occurred to me: We could provide the CodeMirrorWikiEditor class as its own resource module that doesn't require ext.wikiEditor, instead allowing you do that on your own by whatever means. Seems like that might make things easier?

(But I didn't quite understand the difference between this and how you implemented this in 1014134 to be used like in T214989#9702978. Which exact part would be delegated to the client?)

The difference is the any IIFE and/or hook usage. With r1019841 we're providing both, and renaming the modules that "do things" to *.init. Hopefully that makes it a bit more clear.

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

[mediawiki/extensions/CodeMirror@master] CM6: Add 'ext.CodeMirror.input' hook; improve code examples

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

r1019841 provides a means to integrate with CodeMirror unrestricted and with full control. r1019853 meanwhile adds a ext.CodeMirror.input hook that gives you a ViewUpdate object. I think between these and the existing hooks, we've satisfied what is being asked for in this task. I don't see a need to expose CodeMirror internals in the other hooks, or to fire input events on the textarea (given the data we supply is not what the input event would normally provide).

Please let me know if there's anything I'm missing. Dynamic configuration and decisions around it will be tackled as part of T359498.

Thank you. Tbh I don't know how to read changes when there are only /dist/ files in the patch and no /src/ changes 😄 A little note: I think we should still provide code without async/await in official examples since it's not supported in ES6.

Thank you. Tbh I don't know how to read changes when there are only /dist/ files in the patch and no /src/ changes 😄

For this repo, changes in dist/ can always be ignored. In this case, the changes to extension.json are the only thing of real relevance.

A little note: I think we should still provide code without async/await in official examples since it's not supported in ES6.

Thanks for saying this. I got excited when I saw your examples, but was also a bit hesitant to change the examples in the docs. MediaWiki officially supports ES6, and for some years now we expressly don't support Internet Explorer for new JavaScript features. So I guess my question is, is there a reason not to use ES6?

Thanks for saying this. I got excited when I saw your examples, but was also a bit hesitant to change the examples in the docs. MediaWiki officially supports ES6, and for some years now we expressly don't support Internet Explorer for new JavaScript features. So I guess my question is, is there a reason not to use ES6?

That's what I'm saying, async/await is not in ES6! It was added only in ES8 😄

image.png (394×833 px, 380 KB)

That's what I'm saying, async/await is not in ES6! It was added only in ES8 😄

Ah! I see. I was more thinking along the lines of the MediaWiki's browser support matrix, but I see now Grade A still includes Safari 11 which is too old. I shall change the examples back :) Thanks for flagging!

I was more thinking along the lines of the MediaWiki's browser support matrix, but I see now Grade A still includes Safari 11 which is too old. I shall change the examples back :) Thanks for flagging!

I'm confusing myself. Safari 11 does support await, just not async. I'm still going to change the examples back though, to be on the safe side.

(I typically am a "live in the future instead of linger in the past" kind of JS engineer, but official code samples should mostly be in the "now" and not in the future)

Change #1019841 merged by jenkins-bot:

[mediawiki/extensions/CodeMirror@master] CM6: Rework ResourceLoader modules

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

Change #1019853 merged by jenkins-bot:

[mediawiki/extensions/CodeMirror@master] CM6: Add 'ext.CodeMirror.input' hook; improve code examples

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

Change #375613 abandoned by MusikAnimal:

[mediawiki/extensions/CodeMirror@master] Add CodeMirror enabled/disabled JS hooks

Reason:

stale

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