Page MenuHomePhabricator

[Regression wmf.17] "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description'
Closed, ResolvedPublic1 Story Points

Description

Go to any Flow board and click on 'Edit description'. If VE is preferred editor for the board description, the following will be displayed: Uncaught TypeError: Cannot read property 'model' of undefined

stop()
ve.adjacentDomPosition()
ve.ce.Document.prototype.getNodeAndOffset()
ve.ce.Surface.prototype.getSelectionState()
ve.ce.Surface.prototype.showModelSelection()
ve.ce.Surface.prototype.focus()
mw.flow.ui.VisualEditorWidget.prototype.focus()
mw.flow.ui.EditorSwitcherWidget.prototype.switchEditor/<()
.Deferred/promise.then/</</<()
jQuery.Callbacks/fire()
jQuery.Callbacks/self.fireWith()
Deferred/promise.then/</</<()
jQuery.Callbacks/fire()
jQuery.Callbacks/self.fireWith()
.Deferred/</deferred[tuple[0]]()
jQuery.Callbacks/fire()
jQuery.Callbacks/self.fireWith()
.Deferred/</deferred[tuple[0]]()
jQuery.Callbacks/self.fireWith()
.Deferred/</deferred[tuple[0]]()
mw.flow.ui.VisualEditorWidget.prototype.createSurface/<()
oo.EventEmitter.prototype.once/wrapper()
oo.EventEmitter.prototype.emit()
 load.php:61
bound ()
 self-hosted:750

Details

Related Gerrit Patches:
mediawiki/extensions/Flow : wmf/1.28.0-wmf.17Protect against target.getSurface() returning null
mediawiki/extensions/Flow : masterProtect against target.getSurface() returning null
VisualEditor/VisualEditor : mastergetNodeAndOffset: check whether the node has a view before using it
VisualEditor/VisualEditor : wmf/1.28.0-wmf.17ve.init.Target: Unset this.surface when destroying it
VisualEditor/VisualEditor : masterve.init.Target: Unset this.surface when destroying it

Event Timeline

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJul 11 2016, 5:31 PM

I've tried it, and I can't reproduce it.

Re-checked in 1.28.0-alpha (4a09161).
A little bit tricky to reproduce - make sure that

  • use Chrome
  • click 'Edit description' - does not matter if VE is default editor; if wikitext editor appears after 'Edit description' is clicked, switch to VE - the Console error should appear.
  • not repeatable after the first display, but the error appears again after refreshing the page.

I can reproduce it but I have no idea what this.model in ve.ce.Selection is supposed to mean. That's a mission for @Catrope

Still present in betalabs 1.28.0-alpha (b5e308c).

Change 307533 had a related patch set uploaded (by DLynch):
getNodeAndOffset: check whether the node has a view before using it

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

Jdforrester-WMF renamed this task from "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description' to [Regression wmf.17] "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description'.Aug 30 2016, 7:09 PM
Jdforrester-WMF assigned this task to DLynch.
Jdforrester-WMF triaged this task as High priority.
Jdforrester-WMF moved this task from To Triage to TR0: Interrupt on the VisualEditor board.
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF set the point value for this task to 1.
Catrope claimed this task.Aug 31 2016, 2:05 AM
Catrope added a subscriber: DLynch.

Turns out this is a race condition in Flow, caused by f2013dfaa363a91f02069990a36574c2d7148ced. Resolving switchingDeferred before focusing allows setContent() to start destroying the editor before the focus call occurs. We previously focused the editor before resolving the promise, but that also broke because callers would disable the editor and undisable it when the promise resolved, meaning the focus call was dropped. I'll see if I can either get this to not focus if the editor is already being destroyed, or to hold off on destroying the editor for a little bit longer.

Change 307676 had a related patch set uploaded (by Catrope):
ve.init.Target: Unset this.surface when destroying it

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

I was wrong again, it was a bug in ve.init.Target which allowed the following to happen:

surfaceA = createSurface();
target.addSurface( surfaceA );
target.setSurface( surfaceA );
target.getSurface(); // returns surfaceA as expected

target.clearSurfaces();
target.getSurface(); // still returns surfaceA, should return null

In the context of ve.init.Target this is weird but not necessarily harmful, but ve.init.mw.Target extends setSurface() to also attach the surface's DOM to the target's. This leads to the assumption that target.getSurface().$element is always attached to target.$element (or attached to anything at all), but when clearSurfaces() destroys all surfaces, this assumption is violated: the surface is no longer attached but getSurface() still returns it.

The attached patch fixes this by making ve.init.Target unset this.surface if it's going to destroy it.

Change 307677 had a related patch set uploaded (by Catrope):
Protect against target.getSurface() returning null

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

Change 307676 merged by jenkins-bot:
ve.init.Target: Unset this.surface when destroying it

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

Change 307710 had a related patch set uploaded (by Paladox):
ve.init.Target: Unset this.surface when destroying it

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

Change 307710 merged by jenkins-bot:
ve.init.Target: Unset this.surface when destroying it

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

Change 307533 abandoned by DLynch:
getNodeAndOffset: check whether the node has a view before using it

Reason:
Unneeded, given 307676.

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

Change 307869 had a related patch set uploaded (by Paladox):
Protect against target.getSurface() returning null

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

Change 307677 merged by jenkins-bot:
Protect against target.getSurface() returning null

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

Change 307869 merged by jenkins-bot:
Protect against target.getSurface() returning null

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

Mentioned in SAL [2016-09-02T00:00:43Z] <dereckson@tin> Synchronized php-1.28.0-wmf.17/extensions/Flow/modules/flow/ui/widgets/editor/editors/mw.flow.ui.VisualEditorWidget.js: Flow Fixes related to Visual Editor (T138356 and T139972) (duration: 00m 45s)

Checked in betalabs - no such error 9or any errors are displayed).

@Catrope testwiki 1.28.0-wmf.18 (3750c8a) displays e.g. for https://test.wikipedia.org/wiki/Talk:ET43. In fact, it's displayed for each to edit (to start a new topic/post/board description). Hopefully, it's just "residues" of some previously reported problems with VE?

TypeError: Cannot read property 'length' of null
   at LanguageCategoryDisplay.buildQuicklist (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:29:134)
   at LanguageCategoryDisplay.render (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:25:845)
   at new LanguageCategoryDisplay (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:25:2)
   at HTMLDivElement.eval (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:30:629)
   at Function.each (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:5:235)
   at jQuery.each (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:2:222)
   at jQuery.$.fn.lcd (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:30:498)
   at ULS.listen (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:21:389)
   at new ULS (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:19:648)
   at HTMLAnchorElement.eval (eval at <anonymous> (https://test.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=07zvr6j:4:681), <anonymous>:23:270)

Seems fixed from my testing too?

Jdforrester-WMF closed this task as Resolved.Sep 12 2016, 7:08 PM
Restricted Application added projects: Growth-Team, User-Ryasmeen. · View Herald TranscriptSep 2 2018, 10:55 PM