r102675 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r102674‎ \| r102675 \| r102676 >
Date:	19:26, 10 November 2011
Author:	tparscal
Status:	deferred
Tags:
Comment:	Added es.DocumentLeafNode, which like es.DocumentBranchNode is a mixin-like class (we may want to switch to using a more natural composition mechanism than es.extendClass in the future) - now es.DocumentNode also has an abstract method called hasChildren which returns a boolean and can indicate if a node is a leaf or a branch.
Modified paths:	/trunk/extensions/VisualEditor/demo/index.html (modified) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentBranchNode.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentLeafNode.js (added) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelBranchNode.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelLeafNode.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentNode.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentViewBranchNode.js (modified) (history) /trunk/extensions/VisualEditor/modules/es/bases/es.DocumentViewLeafNode.js (modified) (history) /trunk/extensions/VisualEditor/tests/es/index.html (modified) (history)

Diff [purge]

Index: trunk/extensions/VisualEditor/tests/es/index.html
—	—	@@ -23,8 +23,9 @@
24	24	<!-- Bases -->
25	25	<script src="../../modules/es/bases/es.EventEmitter.js"></script>
26	26	<script src="../../modules/es/bases/es.DocumentNode.js"></script>
	27	+ <script src="../../modules/es/bases/es.DocumentModelNode.js"></script>
27	28	<script src="../../modules/es/bases/es.DocumentBranchNode.js"></script>
28		~~- <script src="../../modules/es/bases/es.DocumentModelNode.js"></script>~~
	29	+ <script src="../../modules/es/bases/es.DocumentLeafNode.js"></script>
29	30	<script src="../../modules/es/bases/es.DocumentModelBranchNode.js"></script>
30	31	<script src="../../modules/es/bases/es.DocumentModelLeafNode.js"></script>
31	32
Index: trunk/extensions/VisualEditor/demo/index.html
—	—	@@ -69,8 +69,9 @@
70	70	<!-- Bases -->
71	71	<script src="../modules/es/bases/es.EventEmitter.js"></script>
72	72	<script src="../modules/es/bases/es.DocumentNode.js"></script>
	73	+ <script src="../modules/es/bases/es.DocumentModelNode.js"></script>
73	74	<script src="../modules/es/bases/es.DocumentBranchNode.js"></script>
74		~~- <script src="../modules/es/bases/es.DocumentModelNode.js"></script>~~
	75	+ <script src="../modules/es/bases/es.DocumentLeafNode.js"></script>
75	76	<script src="../modules/es/bases/es.DocumentModelBranchNode.js"></script>
76	77	<script src="../modules/es/bases/es.DocumentModelLeafNode.js"></script>
77	78	<script src="../modules/es/bases/es.DocumentViewNode.js"></script>
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentViewLeafNode.js
—	—	@@ -4,12 +4,14 @@
5	5	* @class
6	6	* @abstract
7	7	* @constructor
	8	+ * @extends {es.DocumentLeafNode}
8	9	* @extends {es.DocumentViewNode}
9	10	* @param model {es.ModelNode} Model to observe
10	11	* @param {jQuery} [$element] Element to use as a container
11	12	*/
12	13	es.DocumentViewLeafNode = function( model, $element ) {
13	14	// Inheritance
	15	+ es.DocumentLeafNode.call( this );
14	16	es.DocumentViewNode.call( this, model, $element );
15	17
16	18	// Properties
—	—	@@ -80,4 +82,5 @@
81	83
82	84	/* Inheritance */
83	85
	86	+es.extendClass( es.DocumentViewLeafNode, es.DocumentLeafNode );
84	87	es.extendClass( es.DocumentViewLeafNode, es.DocumentViewNode );
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentLeafNode.js
—	—	@@ -0,0 +1,23 @@
	2	+/**
	3	+ * Creates an es.DocumentLeafNode object.
	4	+ *
	5	+ * @class
	6	+ * @abstract
	7	+ * @constructor
	8	+ */
	9	+es.DocumentLeafNode = function() {
	10	+ //
	11	+};
	12	+
	13	+/* Methods */
	14	+
	15	+/**
	16	+ * Checks if this node has child nodes.
	17	+ *
	18	+ * @method
	19	+ * @see {es.DocumentNode.prototype.hasChildren}
	20	+ * @returns {Boolean} Whether this node has children
	21	+ */
	22	+es.DocumentLeafNode.prototype.hasChildren = function() {
	23	+ return false;
	24	+};
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelLeafNode.js
—	—	@@ -4,14 +4,15 @@
5	5	* @class
6	6	* @abstract
7	7	* @constructor
	8	+ * @extends {es.DocumentLeafNode}
8	9	* @extends {es.DocumentModelNode}
9		~~- * @extends {es.DocumentNode}~~
10	10	* @param {String} type Symbolic name of node type
11	11	* @param {Object} element Element object in document data
12	12	* @param {Integer} [length] Length of content data in document
13	13	*/
14	14	es.DocumentModelLeafNode = function( type, element, length ) {
15	15	// Inheritance
	16	+ es.DocumentLeafNode.call( this );
16	17	es.DocumentModelNode.call( this, type, element, length );
17	18
18	19	// Properties
—	—	@@ -80,4 +81,5 @@
81	82
82	83	/* Inheritance */
83	84
	85	+es.extendClass( es.DocumentModelLeafNode, es.DocumentLeafNode );
84	86	es.extendClass( es.DocumentModelLeafNode, es.DocumentModelNode );
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentViewBranchNode.js
—	—	@@ -4,15 +4,15 @@
5	5	* @class
6	6	* @abstract
7	7	* @constructor
	8	+ * @extends {es.DocumentBranchNode}
8	9	* @extends {es.DocumentViewNode}
9		~~- * @extends {es.DocumentBranchNode}~~
10	10	* @param model {es.ModelNode} Model to observe
11	11	* @param {jQuery} [$element] Element to use as a container
12	12	*/
13	13	es.DocumentViewBranchNode = function( model, $element, horizontal ) {
14	14	// Inheritance
	15	+ es.DocumentBranchNode.call( this );
15	16	es.DocumentViewNode.call( this, model, $element );
16		~~- es.DocumentBranchNode.call( this );~~
17	17
18	18	// Properties
19	19	this.horizontal = horizontal \|\| false;
—	—	@@ -243,5 +243,5 @@
244	244
245	245	/* Inheritance */
246	246
	247	+es.extendClass( es.DocumentViewBranchNode, es.DocumentBranchNode );
247	248	es.extendClass( es.DocumentViewBranchNode, es.DocumentViewNode );
248		~~-es.extendClass( es.DocumentViewBranchNode, es.DocumentBranchNode );~~
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentBranchNode.js
—	—	@@ -13,6 +13,17 @@
14	14	/* Methods */
15	15
16	16	/**
	17	+ * Checks if this node has child nodes.
	18	+ *
	19	+ * @method
	20	+ * @see {es.DocumentNode.prototype.hasChildren}
	21	+ * @returns {Boolean} Whether this node has children
	22	+ */
	23	+es.DocumentBranchNode.prototype.hasChildren = function() {
	24	+ return true;
	25	+};
	26	+
	27	+/**
17	28	* Gets a list of child nodes.
18	29	*
19	30	* @abstract
—	—	@@ -33,21 +44,21 @@
34	45	*/
35	46	es.DocumentBranchNode.prototype.getRangeFromNode = function( node, shallow ) {
36	47	if ( this.children.length ) {
37		~~- var isBranch;~~
	48	+ var childNode;
38	49	for ( var i = 0, length = this.children.length, left = 0; i < length; i++ ) {
39		~~- if ( this.children[i] === node ) {~~
40		~~- return new es.Range( left, left + this.children[i].getElementLength() );~~
	50	+ childNode = this.children[i];
	51	+ if ( childNode === node ) {
	52	+ return new es.Range( left, left + childNode.getElementLength() );
41	53	}
42		~~- isBranch = typeof this.children[i].getChildren === 'function';~~
43		~~- if ( !shallow && isBranch && this.children[i].getChildren().length ) {~~
44		~~- var range = this.children[i].getRangeFromNode( node );~~
	54	+ if ( !shallow && childNode.hasChildren() && childNode.getChildren().length ) {
	55	+ var range = childNode.getRangeFromNode( node );
45	56	if ( range !== null ) {
46	57	// Include opening of parent
47	58	left++;
48	59	return es.Range.newFromTranslatedRange( range, left );
49	60	}
50	61	}
51		~~- left += this.children[i].getElementLength();~~
	62	+ left += childNode.getElementLength();
52	63	}
53	64	}
54	65	return null;
—	—	@@ -69,19 +80,19 @@
70	81	es.DocumentBranchNode.prototype.getOffsetFromNode = function( node, shallow ) {
71	82	if ( this.children.length ) {
72	83	var offset = 0,
73		~~- isBranch;~~
	84	+ childNode;
74	85	for ( var i = 0, length = this.children.length; i < length; i++ ) {
75		~~- if ( this.children[i] === node ) {~~
	86	+ childNode = this.children[i];
	87	+ if ( childNode === node ) {
76	88	return offset;
77	89	}
78		~~- isBranch = typeof this.children[i].getChildren === 'function';~~
79		~~- if ( !shallow && isBranch && this.children[i].getChildren().length ) {~~
80		~~- var childOffset = this.getOffsetFromNode.call( this.children[i], node );~~
	90	+ if ( !shallow && childNode.hasChildren() && childNode.getChildren().length ) {
	91	+ var childOffset = this.getOffsetFromNode.call( childNode, node );
81	92	if ( childOffset !== -1 ) {
82	93	return offset + 1 + childOffset;
83	94	}
84	95	}
85		~~- offset += this.children[i].getElementLength();~~
	96	+ offset += childNode.getElementLength();
86	97	}
87	98	}
88	99	return -1;
—	—	@@ -105,20 +116,20 @@
106	117	if ( this.children.length ) {
107	118	var nodeOffset = 0,
108	119	nodeLength,
109		~~- isBranch;~~
	120	+ childNode;
110	121	for ( var i = 0, length = this.children.length; i < length; i++ ) {
	122	+ childNode = this.children[i];
111	123	if ( offset == nodeOffset ) {
112		~~- // The requested offset is right before this.children[i],~~
	124	+ // The requested offset is right before childNode,
113	125	// so it's not inside any of this's children, but inside this
114	126	return this;
115	127	}
116		~~- nodeLength = this.children[i].getElementLength();~~
	128	+ nodeLength = childNode.getElementLength();
117	129	if ( offset >= nodeOffset && offset < nodeOffset + nodeLength ) {
118		~~- isBranch = typeof this.children[i].getChildren === 'function';~~
119		~~- if ( !shallow && isBranch && this.children[i].getChildren().length ) {~~
120		~~- return this.getNodeFromOffset.call( this.children[i], offset - nodeOffset - 1 );~~
	130	+ if ( !shallow && childNode.hasChildren() && childNode.getChildren().length ) {
	131	+ return this.getNodeFromOffset.call( childNode, offset - nodeOffset - 1 );
121	132	} else {
122		~~- return this.children[i];~~
	133	+ return childNode;
123	134	}
124	135	}
125	136	nodeOffset += nodeLength;
—	—	@@ -155,7 +166,8 @@
156	167	start = range.start,
157	168	end = range.end,
158	169	startInside,
159		~~- endInside;~~
	170	+ endInside,
	171	+ childNode;
160	172
161	173	if ( start < 0 ) {
162	174	throw 'The start offset of the range is negative';
—	—	@@ -173,20 +185,21 @@
174	186	// This node has children, loop over them
175	187	left = 1; // First offset inside the first child. Offset 0 is before the first child
176	188	for ( i = 0; i < this.children.length; i++ ) {
177		~~- // left <= any offset inside this.children[i] <= right~~
178		~~- right = left + this.children[i].getContentLength();~~
	189	+ childNode = this.children[i];
	190	+ // left <= any offset inside childNode <= right
	191	+ right = left + childNode.getContentLength();
179	192
180	193	if ( start == end && ( start == left - 1 \|\| start == right + 1 ) ) {
181	194	// Empty range outside of any node
182	195	return [];
183	196	}
184	197
185		~~- startInside = start >= left && start <= right; // is the start inside this.children[i]?~~
186		~~- endInside = end >= left && end <= right; // is the end inside this.children[i]?~~
	198	+ startInside = start >= left && start <= right; // is the start inside childNode?
	199	+ endInside = end >= left && end <= right; // is the end inside childNode?
187	200
188	201	if ( startInside && endInside ) {
189		~~- // The range is entirely inside this.children[i]~~
190		~~- if ( shallow \|\| !this.children[i].children ) {~~
	202	+ // The range is entirely inside childNode
	203	+ if ( shallow \|\| !childNode.children ) {
191	204	// For leaf nodes, use the same behavior as for shallow calls.
192	205	// A proper recursive function would let the recursion handle this,
193	206	// but the leaves don't have .selectNodes() because they're not DocumentBranchNodes
—	—	@@ -194,14 +207,14 @@
195	208	// TODO should probably rewrite this recursive function as an iterative function anyway, probably faster
196	209	nodes = [
197	210	{
198		~~- 'node': this.children[i],~~
	211	+ 'node': childNode,
199	212	'range': new es.Range( start - left, end - left ),
200	213	'globalRange': new es.Range( start, end )
201	214	}
202	215	];
203	216	} else {
204		~~- // Recurse into this.children[i]~~
205		~~- nodes = this.children[i].selectNodes( new es.Range( start - left, end - left ) );~~
	217	+ // Recurse into childNode
	218	+ nodes = childNode.selectNodes( new es.Range( start - left, end - left ) );
206	219	// Adjust globalRange
207	220	for ( j = 0; j < nodes.length; j++ ) {
208	221	if ( nodes[j].globalRange !== undefined ) {
—	—	@@ -209,48 +222,48 @@
210	223	}
211	224	}
212	225	}
213		~~- // Since the start and end are both inside this.children[i], we know for sure that we're~~
	226	+ // Since the start and end are both inside childNode, we know for sure that we're
214	227	// done, so return
215	228	return nodes;
216	229	} else if ( startInside ) {
217		~~- // The start is inside this.children[i] but the end isn't~~
218		~~- // Add a range from the start of the range to the end of this.children[i]~~
	230	+ // The start is inside childNode but the end isn't
	231	+ // Add a range from the start of the range to the end of childNode
219	232	nodes.push( {
220		~~- 'node': this.children[i],~~
	233	+ 'node': childNode,
221	234	'range': new es.Range( start - left, right - left ),
222	235	'globalRange': new es.Range( start, right )
223	236	} );
224	237	} else if ( endInside ) {
225		~~- // The end is inside this.children[i] but the start isn't~~
226		~~- // Add a range from the start of this.children[i] to the end of the range~~
	238	+ // The end is inside childNode but the start isn't
	239	+ // Add a range from the start of childNode to the end of the range
227	240	nodes.push( {
228		~~- 'node': this.children[i],~~
	241	+ 'node': childNode,
229	242	'range': new es.Range( 0, end - left ),
230	243	'globalRange': new es.Range( left, end )
231	244	} );
232	245	// We've found the end, so we're done
233	246	return nodes;
234	247	} else if ( end == right + 1 ) {
235		~~- // end is between this.children[i] and this.children[i+1]~~
236		~~- // start is not inside this.children[i], so the selection covers~~
237		~~- // all of this.children[i], then ends~~
238		~~- nodes.push( { 'node': this.children[i] } );~~
	248	+ // end is between childNode and this.children[i+1]
	249	+ // start is not inside childNode, so the selection covers
	250	+ // all of childNode, then ends
	251	+ nodes.push( { 'node': childNode } );
239	252	// We've reached the end so we're done
240	253	return nodes;
241	254	} else if ( start == left - 1 ) {
242		~~- // start is between this.children[i-1] and this.children[i]~~
243		~~- // end is not inside this.children[i], so the selection covers~~
244		~~- // all of this.children[i] and more~~
245		~~- nodes.push( { 'node': this.children[i] } );~~
	255	+ // start is between this.children[i-1] and childNode
	256	+ // end is not inside childNode, so the selection covers
	257	+ // all of childNode and more
	258	+ nodes.push( { 'node': childNode } );
246	259	} else if ( nodes.length > 0 ) {
247		~~- // Neither the start nor the end is inside this.children[i], but nodes is non-empty,~~
248		~~- // so this.children[i] must be between the start and the end~~
	260	+ // Neither the start nor the end is inside childNode, but nodes is non-empty,
	261	+ // so childNode must be between the start and the end
249	262	// Add the entire node, so no range property
250		~~- nodes.push( { 'node': this.children[i] } );~~
	263	+ nodes.push( { 'node': childNode } );
251	264	}
252	265
253	266	// Move left to the start of this.children[i+1] for the next iteration
254		~~- // We use +2 because we need to jump over the offset between this.children[i] and~~
	267	+ // We use +2 because we need to jump over the offset between childNode and
255	268	// this.children[i+1]
256	269	left = right + 2;
257	270	if ( end < left ) {
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentModelBranchNode.js
—	—	@@ -4,16 +4,16 @@
5	5	* @class
6	6	* @abstract
7	7	* @constructor
	8	+ * @extends {es.DocumentBranchNode}
8	9	* @extends {es.DocumentModelNode}
9		~~- * @extends {es.DocumentBranchNode}~~
10	10	* @param {String} type Symbolic name of node type
11	11	* @param {Object} element Element object in document data
12	12	* @param {es.DocumentModelBranchNode[]} [contents] List of child nodes to append
13	13	*/
14	14	es.DocumentModelBranchNode = function( type, element, contents ) {
15	15	// Inheritance
	16	+ es.DocumentBranchNode.call( this );
16	17	es.DocumentModelNode.call( this, type, element, 0 );
17		~~- es.DocumentBranchNode.call( this );~~
18	18
19	19	// Child nodes
20	20	if ( es.isArray( contents ) ) {
—	—	@@ -240,5 +240,5 @@
241	241
242	242	/* Inheritance */
243	243
	244	+es.extendClass( es.DocumentModelBranchNode, es.DocumentBranchNode );
244	245	es.extendClass( es.DocumentModelBranchNode, es.DocumentModelNode );
245		~~-es.extendClass( es.DocumentModelBranchNode, es.DocumentBranchNode );~~
Index: trunk/extensions/VisualEditor/modules/es/bases/es.DocumentNode.js
—	—	@@ -23,6 +23,7 @@
24	24	* Gets the content length.
25	25	*
26	26	* @method
	27	+ * @abstract
27	28	* @returns {Integer} Length of content
28	29	*/
29	30	es.DocumentNode.prototype.getContentLength = function() {
—	—	@@ -33,12 +34,24 @@
34	35	* Gets the element length.
35	36	*
36	37	* @method
	38	+ * @abstract
37	39	* @returns {Integer} Length of content
38	40	*/
39	41	es.DocumentNode.prototype.getElementLength = function() {
40	42	throw 'DocumentNode.getElementLength not implemented in this subclass:' + this.constructor;
41	43	};
42	44
	45	+/**
	46	+ * Checks if this node has child nodes.
	47	+ *
	48	+ * @method
	49	+ * @abstract
	50	+ * @returns {Boolean} Whether this node has children
	51	+ */
	52	+es.DocumentNode.prototype.hasChildren = function() {
	53	+ throw 'DocumentNode.hasChildren not implemented in this subclass:' + this.constructor;
	54	+};
	55	+
43	56	/* Inheritance */
44	57
45	58	es.extendClass( es.DocumentNode, es.EventEmitter );

Status & tagging log

13:58, 25 November 2011 Hashar (talk | contribs) changed the status of r102675 [removed: new added: deferred]