Page MenuHomePhabricator

VisualEditor: JSON.stringify won't work if data contains DOM elements
Closed, ResolvedPublic

Description

Steps to reproduce problem:

  1. Edit https://www.mediawiki.org/wiki/Git/Commit_message_guidelines in VisualEditor
  2. Add some text
  3. "Review and save"
  4. "Something is wrong"
  5. "Report problem"

Expected behaviour:
Make an http request somewhere and report back in the UI.

Actual behaviour:
No http request is made and UI is unresponsive.

console:

Uncaught TypeError: Converting circular structure to JSON
at:

mw.Target.prototype.reportProblem = function(message) {
var ...., report = { title: .., oldid: .., .. };
$.post(mw.config.get('wgVisualEditorConfig').reportProblemURL, {'data': JSON.stringify(report)}, function() {

debug:

var objs = [];
JSON.stringify(a, function (k, v) {

var i;
if (v === Object(v)) {
  i = objs.indexOf(v);
  if (i !== -1) {
    console.log('dupe at #' + i, v);
  }
  objs.push(v);
}
return v;

});
dupe at #66
<p data-parsoid=​"{"dsr":​[0,139,0,0]​}​">​
"The "
<b data-parsoid=​"{"tsr":​[4,7]​,"dsr":​[4,24,3,3]​}​">​commit message​</b>​
" play an important role in revision control systems. They are the first thing other people will see of your commit."
</p>​


Version: unspecified
Severity: critical

Details

Reference
bz47948

Event Timeline

bzimport raised the priority of this task from to Unbreak Now!.Nov 22 2014, 1:28 AM
bzimport set Reference to bz47948.

@Ed: I suspect the circular reference is coming from inside the data model. Can you look into this?

See ve.init.mw.Target.js, method #reportProblem.

This is caused by the linear data now contain DOM elements in alien nodes (and other nodes that store original HTML). We will need to pre-process the data before serialising. We already do this in the unit tests so shouldn't be too difficult.

Related URL: https://gerrit.wikimedia.org/r/62811 (Gerrit Change Id807ccc6ff31d063be815ed4988cb35684adb76a)

Ah, because native DOM element objects have all kinds of "convenience" pointers like "firstChild", "children", "nextSibling", "paretNode", "ownerDocument", which all have the same as well, so its like a circular reference nightmare.