Page MenuHomePhabricator

Editor freezes after copypasting a reference and trying to insert a references list
Closed, ResolvedPublic1 Story Points

Description

Steps to reproduce:

  1. open https://en.wikipedia.org/wiki/Editing?veaction=edit and copy the first footnote ("[1]") to the clipboard
  2. open https://en.wikipedia.org/wiki/Editor_(disambiguation)?veaction=edit and paste in the footnote
  3. click "Insert", "More", "References list"
  4. in the "References list" dialog, click the "Insert" button

Result: The "References list" dialog stays open (with animation running) and can't be closed (neither the "cancel" nor the "insert" button cause a reaction). The JavaScript console in Chromium shows the following:

Uncaught TypeError: undefined is not a functionVM244:588 openAnnotationVM244:486 ve.dm.Converter.openAndCloseAnnotationsVM244:589 ve.ce.ContentBranchNode.getRenderedContentsVM244:591 ve.ce.ContentBranchNode.renderContentsVM244:587 ve.ce.ContentBranchNode.onSpliceVM244:581 VeCeBranchNodeVM244:585 VeCeContentBranchNodeVM244:688 VeCeParagraphNodeVM238:740 oo.Factory.createVM244:583 ve.ce.BranchNode.onSpliceVM244:581 VeCeBranchNodeVM244:687 VeCeInternalItemNodeVM247:95 ve.ce.MWReferencesListNode.updateVM247:92 VeCeMWReferencesListNodeVM238:740 oo.Factory.createVM244:583 ve.ce.BranchNode.onSpliceVM238:737 oo.EventEmitter.emitVM244:363 ve.dm.BranchNode.spliceVM244:223 (anonymous function)VM244:471 ve.dm.Document.rebuildNodesVM244:481 ve.dm.DocumentSynchronizer.synchronizers.rebuildVM244:483 ve.dm.DocumentSynchronizer.synchronizeVM244:384 ve.dm.TransactionProcessor.processVM244:462 ve.dm.Document.commitVM244:437 ve.dm.Surface.changeInternalVM244:436 ve.dm.Surface.changeVM244:440 ve.dm.SurfaceFragment.changeVM244:450 ve.dm.SurfaceFragment.insertContentVM247:176 (anonymous function)VM244:40 (anonymous function)VM244:40 OO.ui.Process.executeVM244:29 OO.ui.Dialog.executeActionVM244:88 OO.ui.ProcessDialog.executeActionVM244:28 OO.ui.Dialog.onActionClickVM238:737 oo.EventEmitter.emitVM238:737 oo.EventEmitter.emitVM244:124 OO.ui.ButtonWidget.onClickload.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=20150218T2…:65 jQuery.event.dispatchload.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=20150218T2…:60 elemData.handle

Event Timeline

Tbayer raised the priority of this task from to Needs Triage.
Tbayer updated the task description. (Show Details)
Tbayer added a project: VisualEditor.
Tbayer added a subscriber: Tbayer.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 19 2015, 8:56 AM

This appears to happen because store indexes in the pasted linear data aren't remapped. The reference contains italicized text, and the italics annotation is item 0 in the pasted document's store. However, store item 0 in the main document's store is, somewhat unusually, an array of DOM nodes stashed there by GeneratedContentNode. So renderContents thinks it's retrieving an annotation from the store but gets an array back, which causes things to go wrong very quickly.

If you first insert a reference list into the second article, then paste the reference in, it'll crash right then and there. Another entertaining symptom of the same bug is the following:

  • Open two new pages in VE
  • On the first page, create a reference with italic text in it
  • On the second page, insert some bold text and a reference list
  • Copypaste the reference from the first page to the second page
  • Observe that the reference list correctly renders the reference contents, except that the text that was supposed to be italic is now bold
Catrope claimed this task.Feb 19 2015, 9:36 AM
Catrope set Security to None.

Change 191557 had a related patch set uploaded (by Catrope):
newFromDocumentInsertion: Also remap store indexes in internalList data

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

Patch-For-Review

Jdforrester-WMF closed this task as Resolved.Feb 23 2015, 4:21 PM
Jdforrester-WMF edited a custom field.

Change 191557 merged by jenkins-bot:
newFromDocumentInsertion: Also remap store indexes in internalList data

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

Checked the scenario described by Catrope in betalabs - seems to be ok.