Page MenuHomePhabricator
Create Task
Maniphest T139972

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

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 Changes in Gerrit:
SubjectRepoBranchLines +/-
Protect against target.getSurface() returning nullmediawiki/extensions/Flowwmf/1.28.0-wmf.17+7 -4
Protect against target.getSurface() returning nullmediawiki/extensions/Flowmaster+7 -4
getNodeAndOffset: check whether the node has a view before using itVisualEditor/VisualEditormaster+5 -1
ve.init.Target: Unset this.surface when destroying itVisualEditor/VisualEditorwmf/1.28.0-wmf.17+5 -0
ve.init.Target: Unset this.surface when destroying itVisualEditor/VisualEditormaster+5 -0
Customize query in gerrit

Related Objects

Event Timeline

Etonkovidova created this task.Jul 11 2016, 5:31 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJul 11 2016, 5:31 PM
Trizek-WMF subscribed.Jul 12 2016, 7:30 AM

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

• jmatazzoni edited projects, added Collab-Team-Q1-July-Sep-2016; removed Collab-Team-2016-Apr-Jun-Q4.Jul 12 2016, 3:15 PM
• jmatazzoni moved this task from Untriaged to Ready for Pickup on the Collab-Team-Q1-July-Sep-2016 board.
Etonkovidova added a comment.Jul 28 2016, 11:13 PM

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.
SBisson added subscribers: Catrope, SBisson.Jul 29 2016, 2:09 PM

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

SBisson merged a task: T119881: View and undo of board description diffs at testwiki:talk:sandbox throws InvalidDataException.Aug 3 2016, 7:27 PM
SBisson added subscribers: Luke081515, • Mattflaschen-WMF, Quiddity.
Etonkovidova added a comment.Aug 16 2016, 8:54 PM

Still present in betalabs 1.28.0-alpha (b5e308c).

• Mattflaschen-WMF unsubscribed.Aug 19 2016, 7:24 PM
Quiddity added a project: VisualEditor.Aug 29 2016, 9:48 PM
• jmatazzoni edited projects, added Collaboration-Team-Triage; removed Collab-Team-Q1-July-Sep-2016.Aug 29 2016, 9:49 PM
• jmatazzoni moved this task from Untriaged to Hot on the Collaboration-Team-Triage board.
gerritbot subscribed.Aug 30 2016, 4:54 PM

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

gerritbot added a project: Patch-For-Review.Aug 30 2016, 4:54 PM
Etonkovidova mentioned this in T138356: Editing comments in flow on mw.org does not work in IE 11.Aug 30 2016, 4:57 PM
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.

gerritbot added a comment.Aug 31 2016, 2:22 AM

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

Catrope added a comment.Aug 31 2016, 2:22 AM

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.

gerritbot added a comment.Aug 31 2016, 2:23 AM

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

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

Catrope edited projects, added Collab-Team-Q1-July-Sep-2016; removed Collaboration-Team-Triage.Aug 31 2016, 2:23 AM
Catrope moved this task from Ready for Pickup to Needs Review on the Collab-Team-Q1-July-Sep-2016 board.
gerritbot added a comment.Aug 31 2016, 4:25 AM

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

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

Paladox subscribed.Aug 31 2016, 8:50 AM
gerritbot added a comment.Aug 31 2016, 8:55 AM

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

gerritbot added a comment.Aug 31 2016, 9:42 AM

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

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

gerritbot added a comment.Aug 31 2016, 3:01 PM

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

gerritbot added a comment.Aug 31 2016, 8:40 PM

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

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

gerritbot added a comment.Aug 31 2016, 8:42 PM

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

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

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-09-06_(1.28.0-wmf.18)).Aug 31 2016, 9:00 PM
gerritbot added a comment.Sep 1 2016, 11:24 PM

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

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

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-08-30_(1.28.0-wmf.17)).Sep 2 2016, 12:00 AM
Stashbot subscribed.Sep 2 2016, 12:00 AM

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)

Catrope moved this task from Needs Review to QA Review on the Collab-Team-Q1-July-Sep-2016 board.Sep 6 2016, 11:45 PM
Etonkovidova added a comment.Sep 7 2016, 6:29 PM

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)
Etonkovidova moved this task from QA Review to Product Review on the Collab-Team-Q1-July-Sep-2016 board.Sep 7 2016, 6:30 PM
Jdforrester-WMF subscribed.Sep 8 2016, 8:31 PM

Seems fixed from my testing too?

Jdforrester-WMF closed this task as Resolved.Sep 12 2016, 7:08 PM
Liuxinyu970226 removed a project: MW-1.28-release (WMF-deploy-2016-08-30_(1.28.0-wmf.17)).Sep 2 2018, 10:55 PM
Restricted Application added projects: Growth-Team, User-Ryasmeen. · View Herald TranscriptSep 2 2018, 10:55 PM
• Petar.petkovic removed a project: Patch-For-Review.Feb 23 2019, 11:50 AM
• Petar.petkovic removed subscribers: Stashbot, gerritbot.
Peachey88 added a project: Regression.Feb 24 2019, 7:06 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits