Page MenuHomePhabricator

[Regression pre-wmf22] Cannot switch to VE in beta - "Cannot read property 'mw' of undefined {stack: (...)" by Citoid
Closed, ResolvedPublic1 Story Points

Description

In betalabs - switching to Edit source and then to VE.

Chrome:
TypeError: Cannot read property 'mw' of undefined {stack: (...), message: "Cannot read property 'mw' of undefined"}message: "Cannot read property 'mw' of undefined"stack: (...)get stack: function () { [native code] }set stack: function () { [native code] }proto: Error

log
(anonymous function)
jQuery.Callbacks.fire
jQuery.Callbacks.self.fireWith
jQuery.Callbacks.self.fire
mw.track
runScript
mw.loader.checkCssHandles
mw.loader.cssHandle
jQuery.Callbacks.fire
jQuery.Callbacks.self.fireWith
jQuery.Callbacks.self.fire
addEmbeddedCSS
(anonymous function)

Event Timeline

Etonkovidova raised the priority of this task from to High.
Etonkovidova updated the task description. (Show Details)
Etonkovidova added a project: VisualEditor.
Etonkovidova added a subscriber: Etonkovidova.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 16 2015, 6:05 PM
Jdforrester-WMF renamed this task from Cannot switch to VE in beta - "Cannot read property 'mw' of undefined {stack: (...)" to [Regression pre-wmf22] Cannot switch to VE in beta - "Cannot read property 'mw' of undefined {stack: (...)" by Citoid.Mar 16 2015, 6:22 PM
Jdforrester-WMF assigned this task to Mooeypoo.
Jdforrester-WMF raised the priority of this task from High to Unbreak Now!.
Jdforrester-WMF added a project: Citoid.
Jdforrester-WMF set Security to None.

FF displays the following:

"MediaWiki error logging: reported an error with id undefined" load.php:135
"Please refer to this error id when reporting an error." load.php:135
"VisualEditor failed to load: Error: Module ext.citoid.visualEditor has failed dependencies" load.php:154
console.trace(): load.php:154
mw.log</log.warn() load.php:154
getTarget/targetPromise<() load.php:54
.Deferred/promise.then/</</<() load.php:47
jQuery.Callbacks/fire() load.php:45
jQuery.Callbacks/self.fireWith() load.php:46
.Deferred/</deferred[tuple[0]]()
jQuery.Callbacks/fire() load.php:45
jQuery.Callbacks/self.fireWith() load.php:46
.Deferred/promise.then/</</<() load.php:47
jQuery.Callbacks/fire() load.php:45
jQuery.Callbacks/self.fireWith() load.php:46
.Deferred/</deferred[tuple[0]]() load.php:47
jQuery.Callbacks/fire() load.php:45
jQuery.Callbacks/self.fireWith() load.php:46
.Deferred/</deferred[tuple[0]]() load.php:47
handlePending() load.php:158
runScript() load.php:160
execute/</checkCssHandles() load.php:161
execute/</cssHandle/<() load.php:161
jQuery.Callbacks/fire() load.php:45
jQuery.Callbacks/self.fireWith() load.php:46
jQuery.Callbacks/self.fire() load.php:46
addEmbeddedCSS() load.php:156
addEmbeddedCSS/<()

Etonkovidova updated the task description. (Show Details)Mar 16 2015, 6:37 PM

This is due to this fix: https://gerrit.wikimedia.org/r/#/c/193026/

Citoid tool is reading ve.init.mw.Target.static.toolbarGroups in order to insert itself into the toolbar. It seems, however, that now Citoid is being loaded before ve.init.mw is available at all, despite the fact that on Citoid extension's end, we have 'ext.visualEditor.mediawiki' as a dependency.

This should really be fixed in the core of the issue -- we should have some hook or event or method to let extensions manipulate the toolbar on load without crashing.

The code crashes on initialization of toolbar Tools; it seems at this point that any external extension that touches the VE toolbar will encounter the same problem.

Mvolz added a subscriber: Mvolz.Mar 16 2015, 7:16 PM

This has been a continuing issue- at least that's why we loaded it "with"
VisualEditor.ext.reference before when it was a dialog; this might be a way
to patch this in the interim (it will just place auto fill below the other
citation tools, which is okay). But yes, an upstream fix would be good.

Yes, I've tried to make the new toolbar change conditional, but the error still pops up on the previous code.
It's then failing on "TypeError: Cannot set property 'CiteFromIdInspectorTool' of undefined" for the line ve.ui.CiteFromIdInspectorTool = function VeUiCiteFromIdInspectorTool( toolGroup, config ) { ... }

Seemingly this happens because ve.ui is not yet defined at this point.

Also, Citoid's dependencies are

	'dependencies ' => array(
		'ext.visualEditor.mwreference',
		'json',
		'ext.visualEditor.mediawiki'
	),

Which should have resulted in loading Citoid after ve.mw.init.Target is already loaded. And yet, it isn't.

I also tried to load after the new 'ext.visualEditor.targetLoader' is loaded, but that doesn't help at all either.

I am not sure there can be a quickfix at this point; The new way of loading changes the order in such a way where we need to figure out how to stall extensions that add tools to the UI and define any other UI objects (like inspectors) until after ve.ui is loaded.

Change 197227 had a related patch set uploaded (by Mooeypoo):
Make Citoid's dependencies dependent

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

Apparently I need better glasses.

There was an extra space in the 'dependencies' definition in the Citoid.php file, which made ResourceLoader simply ignore the dependencies completely. It worked before because magic; now that Roan's fix actually loads plugins in order and properly, the error was exposed.

Hooray for an easy fix....

We all need glasses :).

Change 197231 had a related patch set uploaded (by Jforrester):
Make Citoid's dependencies dependent

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

Change 197227 merged by jenkins-bot:
Make Citoid's dependencies dependent

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

Change 197231 merged by jenkins-bot:
Make Citoid's dependencies dependent

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

Mvolz moved this task from Backlog to Extension on the Citoid board.Mar 18 2015, 3:28 PM
Mvolz closed this task as Resolved.Mar 18 2015, 4:06 PM

The fix works - verified in beta.