Page MenuHomePhabricator

VisualEditor: Fix "Uncaught TypeError: Cannot read property 'params' of undefined 'content'"
Closed, DeclinedPublic

Description

Per http://en.wikipedia.org/wiki/Wikipedia:VisualEditor/Feedback#Content_transclusion_causes_WTF_mode

If trying to add a template, only adding content (empty) and click apply, following error is thrown:

TypeError: content is undefined
http://bits.wikimedia.org/static-1.22wmf11/extensions/VisualEditor/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js
Line 192


Version: unspecified
Severity: normal

Details

Reference
bz52198

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 1:58 AM
bzimport set Reference to bz52198.
  1. Insert Transclusion
  2. Ignore "New template" panel in the dialog and go to "+ [] Add content"
  3. Type "x"
  4. Apply changes
  5. > Uncaught TypeError: content is undefined

http://bits.wikimedia.org/static-1.22wmf11/extensions/VisualEditor/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js:192

ve.dm.MWTransclusionNode.prototype.getWikitext = function() {
    var i, len, part, template, param, content = this.getAttribute('mw'), wikitext = '';
if (content.params) {
    content = {'parts': [{'template': content}]};
}

Uncaught TypeError: Cannot read property 'params' of undefined
ve.dm.MWTransclusionNode.getWikitext
ve.ce.MWTransclusionNode.generateContents
ve.ce.GeneratedContentNode.onUpdate
VeCeGeneratedContentNode
VeCeMWTransclusionNode
VeCeMWTransclusionInlineNode
ve.Factory.create
ve.ce.BranchNode.onSplice
ve.ce.ContentBranchNode.onSplice
oo.EventEmitter.emit
ve.dm.BranchNode.splice
ve.insertIntoArray
ve.dm.Document.rebuildNodes
ve.dm.DocumentSynchronizer.synchronizers.rebuild
ve.dm.DocumentSynchronizer.synchronize
ve.dm.TransactionProcessor.process
ve.dm.Document.commit
ve.dm.Surface.change
ve.dm.SurfaceFragment.insertContent
ve.ui.MWTransclusionDialog.onClose
ve.ui.Window.close
(anonymous function)
proxy

if (content.params) {

If we had used coffescript instead of plain JS, it would be so simple to write that as:
if content?.params

...

Now I assume we'll need to do it manually as

if (typeof content !== "undefined" && content !== null && content.params) {
  ...
}

(In reply to comment #2)

if (content.params) {

If we had used coffescript instead of plain JS, it would be so simple to
write
that as:
if content?.params

...

Now I assume we'll need to do it manually as

if (typeof content !== "undefined" && content !== null && content.params) {
  ...
}

It's much easier than that, you can just do if ( content && content.params ) . No need to use typeof and string comparisons and all that :)

Really, though, content shouldn't be undefined in the first place.

(In reply to comment #3)

(In reply to comment #2)

if (content.params) {

If we had used coffescript instead of plain JS, it would be so simple to
write
that as:
if content?.params

...

Now I assume we'll need to do it manually as

if (typeof content !== "undefined" && content !== null && content.params) {
  ...
}

It's much easier than that, you can just do if ( content && content.params )
.
No need to use typeof and string comparisons and all that :)

Really, though, content shouldn't be undefined in the first place.

Indeed, guarding it with an if statement for content? itself is undesirable because the error is elsewhere.

Doesn't give a error anymore, but API is returning:

{"warnings":{"main":{"*":"Unrecognized parameter: 'token'"}},"visualeditor":{"result":"success","content":"<p>x</p>\n\n"}}

(In reply to comment #1)

  1. Insert Transclusion
  2. Ignore "New template" panel in the dialog and go to "+ [] Add content"
  3. Type "x"
  4. Apply changes
  1. Show changes
  2. Review your changes

    Foo

+ x

Bar

All works as expected on latest master, and no errors.

(In reply to comment #5)

Doesn't give a error anymore, but API is returning:

{"warnings":{"main":{"*":"Unrecognized parameter:
'token'"}},"visualeditor":{"result":"success","content":"<p>x</p>\n\n"}}

That warning is unrelated. We're passively passing the authentication token to API requests, even though some API requests don't require a token. This is technical debt we should clean up.

Closing bug as fixed/worksforme.