r105209 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r105208‎ \| r105209 \| r105210 >
Date:	19:41, 5 December 2011
Author:	tparscal
Status:	deferred
Tags:
Comment:	* Moved getContent and getText from leaf nodes to document model nodes * Renamed getContent to getContentData * Renamed getText to getContentText * Added getElementData
Modified paths:	/trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelLeafNode.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelNode.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/models/es.DocumentModel.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/views/es.ContentView.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/views/es.SurfaceView.js (modified) (history) /trunk/extensions/VisualEditor/tests/es/es.DocumentModel.test.js (modified) (history) /trunk/extensions/VisualEditor/tests/es/es.TransactionProcessor.test.js (modified) (history)

Diff [purge]

Index: trunk/extensions/VisualEditor/tests/es/es.TransactionProcessor.test.js
—	—	@@ -94,7 +94,7 @@
95	95
96	96	// Test 6
97	97	deepEqual(
98		~~- documentModel.getChildren()[0].getContent(),~~
	98	+ documentModel.getChildren()[0].getContentData(),
99	99	[
100	100	'a',
101	101	['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }],
—	—	@@ -120,7 +120,7 @@
121	121
122	122	// Test 8
123	123	deepEqual(
124		~~- documentModel.getChildren()[0].getContent(),~~
	124	+ documentModel.getChildren()[0].getContentData(),
125	125	[
126	126	'a',
127	127	['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }],
—	—	@@ -145,7 +145,7 @@
146	146
147	147	// Test 10
148	148	deepEqual(
149		~~- documentModel.getChildren()[0].getContent(),~~
	149	+ documentModel.getChildren()[0].getContentData(),
150	150	['a'],
151	151	'commit keeps model tree up to date with removals'
152	152	);
—	—	@@ -166,7 +166,7 @@
167	167
168	168	// Test 12
169	169	deepEqual(
170		~~- documentModel.getChildren()[0].getContent(),~~
	170	+ documentModel.getChildren()[0].getContentData(),
171	171	[
172	172	'a',
173	173	['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }],
—	—	@@ -197,14 +197,14 @@
198	198
199	199	// Test 14
200	200	deepEqual(
201		~~- documentModel.getChildren()[0].getContent(),~~
	201	+ documentModel.getChildren()[0].getContentData(),
202	202	['a'],
203	203	'commit keeps model tree up to date with paragraph split (paragraph 1)'
204	204	);
205	205
206	206	// Test 15
207	207	deepEqual(
208		~~- documentModel.getChildren()[1].getContent(),~~
	208	+ documentModel.getChildren()[1].getContentData(),
209	209	[
210	210	['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }],
211	211	['c', { 'type': 'textStyle/italic', 'hash': '{"type":"textStyle/italic"}' }]
—	—	@@ -228,7 +228,7 @@
229	229
230	230	// Test 17
231	231	deepEqual(
232		~~- documentModel.getChildren()[0].getContent(),~~
	232	+ documentModel.getChildren()[0].getContentData(),
233	233	[
234	234	'a',
235	235	['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }],
—	—	@@ -272,21 +272,21 @@
273	273
274	274	// Test 21
275	275	deepEqual(
276		~~- documentModel.children[1].children[0].children[0].children[1].children[0].children[0].getContent(),~~
	276	+ documentModel.children[1].children[0].children[0].children[1].children[0].children[0].getContentData(),
277	277	[ 'f' ],
278	278	'removal keeps model tree up to date with list item merge (first list item)'
279	279	);
280	280
281	281	// Test 22
282	282	deepEqual(
283		~~- documentModel.children[1].children[0].children[0].children[1].children[1].children[0].getContent(),~~
	283	+ documentModel.children[1].children[0].children[0].children[1].children[1].children[0].getContentData(),
284	284	[ 'g' ],
285	285	'removal keeps model tree up to date with list item merge (second list item)'
286	286	);
287	287
288	288	// Test 23
289	289	deepEqual(
290		~~- documentModel.children[2].getContent(),~~
	290	+ documentModel.children[2].getContentData(),
291	291	[ 'h' ],
292	292	'rollback keeps model tree up to date with list item split (final paragraph)'
293	293	);
—	—	@@ -322,28 +322,28 @@
323	323
324	324	// Test 26
325	325	deepEqual(
326		~~- documentModel.children[1].children[0].children[0].children[1].children[0].children[0].getContent(),~~
	326	+ documentModel.children[1].children[0].children[0].children[1].children[0].children[0].getContentData(),
327	327	[ 'e' ],
328	328	'rollback keeps model tree up to date with list item split (first list item)'
329	329	);
330	330
331	331	// Test 27
332	332	deepEqual(
333		~~- documentModel.children[1].children[0].children[0].children[1].children[1].children[0].getContent(),~~
	333	+ documentModel.children[1].children[0].children[0].children[1].children[1].children[0].getContentData(),
334	334	[ 'f' ],
335	335	'rollback keeps model tree up to date with list item split (second list item)'
336	336	);
337	337
338	338	// Test 28
339	339	deepEqual(
340		~~- documentModel.children[1].children[0].children[0].children[1].children[2].children[0].getContent(),~~
	340	+ documentModel.children[1].children[0].children[0].children[1].children[2].children[0].getContentData(),
341	341	[ 'g' ],
342	342	'rollback keeps model tree up to date with list item split (third list item)'
343	343	);
344	344
345	345	// Test 29
346	346	deepEqual(
347		~~- documentModel.children[2].getContent(),~~
	347	+ documentModel.children[2].getContentData(),
348	348	[ 'h' ],
349	349	'rollback keeps model tree up to date with list item split (final paragraph)'
350	350	);
Index: trunk/extensions/VisualEditor/tests/es/es.DocumentModel.test.js
—	—	@@ -85,50 +85,50 @@
86	86	);
87	87	} );
88	88
89		~~-test( 'es.DocumentModel.getContent', 6, function() {~~
	89	+test( 'es.DocumentModel.getContentData', 6, function() {
90	90	var documentModel = es.DocumentModel.newFromPlainObject( esTest.obj ),
91	91	childNodes = documentModel.getChildren();
92	92
93	93	// Test 1
94	94	deepEqual(
95		~~- childNodes[0].getContent( new es.Range( 1, 3 ) ),~~
	95	+ childNodes[0].getContentData( new es.Range( 1, 3 ) ),
96	96	[
97	97	['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }],
98	98	['c', { 'type': 'textStyle/italic', 'hash': '{"type":"textStyle/italic"}' }]
99	99	],
100		~~- 'getContent can return an ending portion of the content'~~
	100	+ 'getContentData can return an ending portion of the content'
101	101	);
102	102
103	103	// Test 2
104	104	deepEqual(
105		~~- childNodes[0].getContent( new es.Range( 0, 2 ) ),~~
	105	+ childNodes[0].getContentData( new es.Range( 0, 2 ) ),
106	106	['a', ['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }]],
107		~~- 'getContent can return a beginning portion of the content'~~
	107	+ 'getContentData can return a beginning portion of the content'
108	108	);
109	109
110	110	// Test 3
111	111	deepEqual(
112		~~- childNodes[0].getContent( new es.Range( 1, 2 ) ),~~
	112	+ childNodes[0].getContentData( new es.Range( 1, 2 ) ),
113	113	[['b', { 'type': 'textStyle/bold', 'hash': '{"type":"textStyle/bold"}' }]],
114		~~- 'getContent can return a middle portion of the content'~~
	114	+ 'getContentData can return a middle portion of the content'
115	115	);
116	116
117	117	// Test 4
118	118	try {
119		~~- childNodes[0].getContent( new es.Range( -1, 3 ) );~~
	119	+ childNodes[0].getContentData( new es.Range( -1, 3 ) );
120	120	} catch ( negativeIndexError ) {
121		~~- ok( true, 'getContent throws exceptions when given a range with start < 0' );~~
	121	+ ok( true, 'getContentData throws exceptions when given a range with start < 0' );
122	122	}
123	123
124	124	// Test 5
125	125	try {
126		~~- childNodes[0].getContent( new es.Range( 0, 4 ) );~~
	126	+ childNodes[0].getContentData( new es.Range( 0, 4 ) );
127	127	} catch ( outOfRangeError ) {
128		~~- ok( true, 'getContent throws exceptions when given a range with end > length' );~~
	128	+ ok( true, 'getContentData throws exceptions when given a range with end > length' );
129	129	}
130	130
131	131	// Test 6
132		~~- deepEqual( childNodes[2].getContent(), ['h'], 'Content can be extracted from nodes' );~~
	132	+ deepEqual( childNodes[2].getContentData(), ['h'], 'Content can be extracted from nodes' );
133	133	} );
134	134
135	135	test( 'es.DocumentModel.getIndexOfAnnotation', 3, function() {
Index: trunk/extensions/VisualEditor/modules/es/models/es.DocumentModel.js
—	—	@@ -613,13 +613,28 @@
614	614	};
615	615
616	616	/**
	617	+ * Gets the element data of a node.
	618	+ *
	619	+ * @method
	620	+ * @param {es.DocumentModelNode} node Node to get element data for
	621	+ */
	622	+es.DocumentModel.prototype.getContentDataFromNode = function( node ) {
	623	+ var length = node.getElementLength();
	624	+ var offset = this.getOffsetFromNode( node );
	625	+ if ( offset !== -1 ) {
	626	+ return this.data.slice( offset, offset + length );
	627	+ }
	628	+ return null;
	629	+};
	630	+
	631	+/**
617	632	* Gets the content data of a node.
618	633	*
619	634	* @method
620	635	* @param {es.DocumentModelNode} node Node to get content data for
621	636	* @returns {Array\|null} List of content and elements inside node or null if node is not found
622	637	*/
623		~~-es.DocumentModel.prototype.getContentFromNode = function( node, range ) {~~
	638	+es.DocumentModel.prototype.getContentDataFromNode = function( node, range ) {
624	639	var length = node.getContentLength();
625	640	if ( range ) {
626	641	range.normalize();
—	—	@@ -1201,7 +1216,7 @@
1202	1217	txs.push( this.prepareInsertion(
1203	1218	nodeOffset,
1204	1219	[ { 'type': type, 'attributes': attributes } ]
1205		~~- .concat( nodes[i].getContent() )~~
	1220	+ .concat( nodes[i].getContentData() )
1206	1221	.concat( [ { 'type': '/' + type } ] )
1207	1222	) );
1208	1223	}
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelNode.js
—	—	@@ -218,6 +218,71 @@
219	219	return null;
220	220	};
221	221
	222	+/**
	223	+ * Gets all element data, including the element opening, closing and it's contents.
	224	+ *
	225	+ * @method
	226	+ * @returns {Array} Element data
	227	+ */
	228	+es.DocumentModelNode.prototype.getElementData = function() {
	229	+ // Get reference to the document, which might be this node but otherwise should be this.root
	230	+ var root = this.type === 'document' ?
	231	+ this : ( this.root && this.root.type === 'document' ? this.root : null );
	232	+ if ( root ) {
	233	+ return root.getElementDataFromNode( this );
	234	+ }
	235	+ return [];
	236	+};
	237	+
	238	+/**
	239	+ * Gets content data within a given range.
	240	+ *
	241	+ * @method
	242	+ * @param {es.Range} [range] Range of content to get
	243	+ * @returns {Array} Content data
	244	+ */
	245	+es.DocumentModelNode.prototype.getContentData = function( range ) {
	246	+ // Get reference to the document, which might be this node but otherwise should be this.root
	247	+ var root = this.type === 'document' ?
	248	+ this : ( this.root && this.root.type === 'document' ? this.root : null );
	249	+ if ( root ) {
	250	+ return root.getContentDataFromNode( this, range );
	251	+ }
	252	+ return [];
	253	+};
	254	+
	255	+/**
	256	+ * Gets plain text version of the content within a specific range.
	257	+ *
	258	+ * Two newlines are inserted between leaf nodes.
	259	+ *
	260	+ * TODO: Maybe do something more adaptive with newlines
	261	+ *
	262	+ * @method
	263	+ * @param {es.Range} [range] Range of text to get
	264	+ * @returns {String} Text within given range
	265	+ */
	266	+es.DocumentModelNode.prototype.getContentText = function( range ) {
	267	+ var content = this.getContentData( range );
	268	+ // Copy characters
	269	+ var text = '',
	270	+ element = false;
	271	+ for ( var i = 0, length = content.length; i < length; i++ ) {
	272	+ if ( typeof content[i] === 'object' ) {
	273	+ if ( i ) {
	274	+ element = true;
	275	+ }
	276	+ } else {
	277	+ if ( element ) {
	278	+ text += '\n\n';
	279	+ element = false;
	280	+ }
	281	+ text += typeof content[i] === 'string' ? content[i] : content[i][0];
	282	+ }
	283	+ }
	284	+ return text;
	285	+};
	286	+
222	287	/* Inheritance */
223	288
224	289	es.extendClass( es.DocumentModelNode, es.DocumentNode );
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelLeafNode.js
—	—	@@ -34,50 +34,10 @@
35	35	if ( this.element && this.element.attributes ) {
36	36	obj.attributes = es.copyObject( this.element.attributes );
37	37	}
38		~~- obj.content = es.DocumentModel.getExpandedContentData( this.getContent() );~~
	38	+ obj.content = es.DocumentModel.getExpandedContentData( this.getContentData() );
39	39	return obj;
40	40	};
41	41
42		-/**
43		~~- * Gets the content length.~~
44		- *
45		~~- * FIXME: This method makes assumptions that a node with a data property is a DocumentModel, which~~
46		~~- * may be an issue if sub-classes of DocumentModelLeafNode other than DocumentModel have a data property~~
47		~~- * as well. A safer way of determining this would be helpful in preventing future bugs.~~
48		- *
49		~~- * @method~~
50		~~- * @param {es.Range} [range] Range of content to get~~
51		~~- * @returns {Integer} Length of content~~
52		~~- */~~
53		~~-es.DocumentModelLeafNode.prototype.getContent = function( range ) {~~
54		~~- // Find root~~
55		~~- var root = this.data ? this : ( this.root && this.root.data ? this.root : null );~~
56		~~- if ( root ) {~~
57		~~- return root.getContentFromNode( this, range );~~
58		~~- }~~
59		~~- return [];~~
60		~~-};~~
61		-
62		-/**
63		~~- * Gets plain text version of the content within a specific range.~~
64		- *
65		~~- * @method~~
66		~~- * @param {es.Range} [range] Range of text to get~~
67		~~- * @returns {String} Text within given range~~
68		~~- */~~
69		~~-es.DocumentModelLeafNode.prototype.getText = function( range ) {~~
70		~~- var content = this.getContent( range );~~
71		~~- // Copy characters~~
72		~~- var text = '';~~
73		~~- for ( var i = 0, length = content.length; i < length; i++ ) {~~
74		~~- // If not using in IE6 or IE7 (which do not support array access for strings) use this..~~
75		~~- // text += this.data[i][0];~~
76		~~- // Otherwise use this...~~
77		~~- text += typeof content[i] === 'string' ? content[i] : content[i][0];~~
78		~~- }~~
79		~~- return text;~~
80		~~-};~~
81		-
82	42	/* Inheritance */
83	43
84	44	es.extendClass( es.DocumentModelLeafNode, es.DocumentLeafNode );
Index: trunk/extensions/VisualEditor/modules/es/views/es.SurfaceView.js
—	—	@@ -589,7 +589,7 @@
590	590	this.model.transact( tx, true );
591	591	} else {
592	592	tx = this.model.getDocument().prepareInsertion(
593		~~- targetOffset, sourceNode.model.getContent()~~
	593	+ targetOffset, sourceNode.model.getContentData()
594	594	);
595	595	this.model.transact( tx, true );
596	596
Index: trunk/extensions/VisualEditor/modules/es/views/es.ContentView.js
—	—	@@ -500,7 +500,7 @@
501	501	* the words.
502	502	*/
503	503	// Get and cache a copy of all content, the make a plain-text version of the cached content
504		~~- var data = this.contentCache = this.model.getContent(),~~
	504	+ var data = this.contentCache = this.model.getContentData(),
505	505	text = '';
506	506	for ( var i = 0, length = data.length; i < length; i++ ) {
507	507	text += typeof data[i] === 'string' ? data[i] : data[i][0];
—	—	@@ -712,7 +712,7 @@
713	713	$line[0].innerHTML = this.getHtml( range );
714	714	// Overwrite/append line information
715	715	this.lines[rs.line] = {
716		~~- 'text': this.model.getText( range ),~~
	716	+ 'text': this.model.getContentText( range ),
717	717	'range': range,
718	718	'width': $line.outerWidth(),
719	719	'height': $line.outerHeight(),

Status & tagging log

12:28, 7 December 2011 Siebrand (talk | contribs) changed the status of r105209 [removed: new added: deferred]