wikimedia/mediawiki-extensions-VisualEditor
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/VisualEditor synced 2024-11-28 16:20:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

There's no such thing as ghosts, just sneaky array references

Browse source 
When a document is created, it should take it upon itself to make sure it has a new reference to the data using slice, not place this on the caller. Callers that do not use slice will often find strange and mysterious things going on and not know why. The real reason is that multiple documents sharing a reference to the same data array leads to seriously messed up behavior.

Change-Id: Ic4e25fcd9bf3f41a805003520a8f38e2768f5dbf
This commit is contained in:
Trevor Parscal 2012-08-16 14:18:50 -07:00
parent 297b4568d8
commit 264b97df7f
3 changed files with 11 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
modules/ve/dm/ve.dm.Document.js
View file

@ -8,6 +8,13 @@
/**
* DataModel document.
*
* WARNING: Data passed into a new document will be sliced, creating a shallow copy. This is done to
* prevent multiple documents sharing references to the same data, which causes very strange and
* difficult to diagnose issues. By slicing by default, this issue is dealt with automatically. This
* comes at a price however. While a slice is much faster than a deep copy, it may still be a
* problem with really big data sets. We do not know that this is an issue yet, but consider it a
* likely area to cause performance problems in the future.
*
* @class
* @extends {ve.Document}
* @constructor
@ -20,7 +27,7 @@ ve.dm.Document = function ( data, parentDocument ) {
// Properties
this.parentDocument = parentDocument;
this.data = data || [];
this.data = ve.isArray( data ) ? data.slice( 0 ) : [];
// Initialization
/*

2
modules/ve/test/dm/ve.dm.Document.test.js
View file

@ -898,7 +898,7 @@ QUnit.test( 'isContentData', 1, function ( assert ) {
QUnit.test( 'rebuildNodes', function ( assert ) {
var tree,
doc = new ve.dm.Document( ve.dm.example.data.slice( 0 ) ),
doc = new ve.dm.Document( ve.dm.example.data ),
documentNode = doc.getDocumentNode();
// Rebuild table without changes
doc.rebuildNodes( documentNode, 1, 1, 5, 32 );

4
modules/ve/test/dm/ve.dm.DocumentSynchronizer.test.js
View file

@ -10,13 +10,13 @@ QUnit.module( 've.dm.DocumentSynchronizer' );
/* Tests */
QUnit.test( 'getDocument', 1, function ( assert ) {
var doc = new ve.dm.Document( ve.dm.example.data.slice( 0 ) ),
var doc = new ve.dm.Document( ve.dm.example.data ),
ds = new ve.dm.DocumentSynchronizer( doc );
assert.strictEqual( ds.getDocument(), doc );
} );
QUnit.test( 'synchronize', 2, function ( assert ) {
var doc = new ve.dm.Document( ve.dm.example.data.slice( 0 ) ),
var doc = new ve.dm.Document( ve.dm.example.data ),
ds = new ve.dm.DocumentSynchronizer( doc );
// Annotate "a" with bold formatting