Page MenuHomePhabricator

[Regression] Disabled CodeEditor prevents toolbar functions to work
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. Open any page CodeEditor works on for editing, e.g. https://de.wikipedia.org/w/index.php?title=Modul:Foo&action=edit&redlink=1
  2. Click the icon to disable CodeEditor.
  3. Use any function of the toolbar, e.g. insert a special character, or try to search and replace something.

Expected result: The special character should be inserted, search&replace be executed.
Actual result: An error is thrown: TypeError: context.codeEditor is undefined

This might be related to https://gerrit.wikimedia.org/r/139690


Version: unspecified
Severity: major

Details

Reference
bz68556

Event Timeline

bzimport raised the priority of this task from to High.
bzimport set Reference to bz68556.
Schnark created this task.Jul 25 2014, 8:57 AM

Are we sure that that is a regression ? Because if a take a peek at textSelection, it seems like it would have done that in the past as well..

Will have to investigate a bit.

(In reply to Derk-Jan Hartman from comment #1)

Are we sure that that is a regression ? Because if a take a peek at
textSelection, it seems like it would have done that in the past as well..

Yes, I am sure. I have CodeEditor disabled, and use several user scripts that use the textSelection API, and they just broke yesterday.

After a quick look: When you disable CodeEditor, context.$iframe is set to undefined [1], which previously was recognized by textSeclection as to use the default methods, while the mentioned change removed that check. So now the code always tries to call the methods provided by CodeEditor, but these fail when it is disabled.

[1] https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FCodeEditor/56b9e4c350c122e105949b0faefa25f17253335b/modules%2Fjquery.codeEditor.js#L276

TheDJ added a comment.Jul 26 2014, 1:23 PM

Bingo got it. Now to find a better way to fix this....
and more importantly.. document it...

TheDJ added a comment.Jul 28 2014, 7:45 PM

Yuck...

This is really all down to the fact that the textSelection api was once a part of the WikiEditor api.

That separation was never really properly finished and that is why in the CodeEditor this api is now 'mixed' up with the WikiEditor api. Add to this that CodeEditor made it possible to enable and disable on the fly and you have one big mess...

So this bug is because I failed to realize that the CodeEditor could be disabled in the same session and that this would 'break' the function mapping in:

http://git.wikimedia.org/blob/mediawiki%2Fextensions%2FCodeEditor.git/ad908b2da64da4083e10e391153112978fb9df8f/modules%2Fjquery.codeEditor.js#L517

There we basically assign the 'textSelection' api functions into the WikiEditor api. The disabling works fine for the WikiEditor API functions, because they listen to the conditional in L508, but the textSelection APIs are no longer part of the wikiEditor api's and stuff start breaking.

I'm thus going to reintroduce iframe check and then submit a new patch that does some very simple API registration to replace the textSelection API if a textfield/area wants to do that.

Change 150009 had a related patch set uploaded by TheDJ:
jquery.textSelection: re-add iframe check due to regression

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

Change 150009 merged by jenkins-bot:
jquery.textSelection: re-add iframe check due to regression

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