Page MenuHomePhabricator

AbuseFilter's CodeEditor embed should fire "codeEditor.configure" hook
Open, Needs TriagePublic

Description

There's codeEditor.configure hook used by CodeEditor ext when Ace editor is loaded to allow Ace configuration. When AbuseFilter's Ace component is loaded no hook is being fired, making it unreasonably difficult to apply Ace configuration with JavaScript.

Tl;dr AbuseFilter with CodeEditor should reflect behavior as in https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/CodeEditor/+/refs/heads/master/modules/jquery.codeEditor.js#408

Event Timeline

While I agree that it might be useful to offer a hook for customizing the editor, there are a couple of important things:

  • AbuseFilter doesn't use anything from CodeEditor. The only reason for the dependency on CodeEditor is that it avoids requiring ace separately from that, but AF would keep working if you install ace separately from CodeEditor. I'd like to keep this behaviour, and not use anything CodeEditor-specific.
  • In light of the above, it doesn't feel right to fire a hook that is specific to CodeEditor.

So perhaps firing a different hook would be a better idea.

That's a fair point as I'm looking at the code. Maybe abuseFilter.ace.loaded would be a better name? I couldn't find any other mw.hook() in AbuseFilter repo, so I'm assuming camel case for the name.

Change 675831 had a related patch set uploaded (by Rail; author: Rail):
[mediawiki/extensions/AbuseFilter@master] Add hook to customize Ace editor embed

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

I decided to go with abuseFilter.configureAce. Tested and it worked just as expected. Hope this change is fine but if it isn't please let me know.