/static/master/extensions/VisualEditor/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.init.js:156 Uncaught TypeError: Cannot read property 'requestPageData' of undefined
mw.libs.ve.targetLoader is defined in ext.visualEditor.targetLoader, of which an mw.loader.using wrapper encloses this line, so… help.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | ori | T108197 Blockers for 1.26wmf18 (released on 2015-08-11) | |||
| Resolved | Catrope | T107979 [Regression pre-wmf18] VisualEditor doesn't load, throwing "Uncaught TypeError: Cannot read property 'requestPageData' of undefined" |
The problem is that there is a race condition between two paths:
Commit 812cb9aa2 acknowledges that this race condition happens sometimes, but did not address it correctly.
DesktopArticleTarget.init is where the mw.libs.ve object is primarily created. The TargetLoader extends this object with an additional .targetLoader property. Since TargetLoader can (and should) arrive independently it means the host object may be missing at that time.
812cb9aa2 added tolerance in TargetLoader by lazy-initializing that object. However that only fixes one of the two exceptions. It prevented TargetLoader from failing for mw.lib.ve being undefined. But it does not prevent DesktopArticleTarget.init from failing on mw.lib.ve.targetLoader (as we're seeing right now) because DesktopArticleTarget.init will still (re-)create the host object as usual and thus blow away the TargetLoader's property.
In the past, the proper solution would've been to add a dependency on DesktopArticleTarget.init for ext.visualEditor.targetLoader so that execution unfolds in the right order (regardless of load order). However since TargetLoader is used by both Desktop and Mobile targets, a dependency is not an option.
In the short-term I'd recommend we add tolerance, similar to 812cb9aa2, in DesktopArticleTarget.init. E.g. lazy-create the object and use $.extend or property assignments for the methods instead of assigning the object as whole. That way pre-existing properties (such as TargetLoader) are preserved.
In the medium-term I'd recommend we restore the fact that mw.libs.ve is owned by a single common module (instead of tolerated from two sides) and merely extended by things like Desktop and TargetLoader. Especially because right now some "public methods" in mw.libs.ve are missing on Mobile because its target doesn't define them (it only loads TargetLoader). E.g. mw.libs.ve.addPlugin and mw.libs.ve.isAvailable are missing on mobile.
Change 229463 had a related patch set uploaded (by Catrope):
DesktopArticleTarget.init: Don't overwrite mw.libs.ve
Change 229463 merged by jenkins-bot:
DesktopArticleTarget.init: Don't overwrite mw.libs.ve
Change 229976 had a related patch set uploaded (by Krinkle):
DesktopArticleTarget.init: Don't overwrite mw.libs.ve
Change 229976 merged by jenkins-bot:
DesktopArticleTarget.init: Don't overwrite mw.libs.ve