User "Catrope" changed the status of MediaWiki.r103377.

Old Status: new
New Status: fixme

User "Catrope" also posted a comment on MediaWiki.r103377.

Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/103377#c26141
Commit summary:

* Added support for inserting content at a structural offset
* Broke rebuildNodes into 2 parts so insert can use just buildNodes when 
inserting whole nodes
* Added getIndexFromOffset to es.DocumentModelBranchNode objects, which returns 
an index of a child node from an offset

Comment:

<pre>
+                       index = parent.getIndexFromOffset( this.cursor );
</pre>
This looks wrong. <code>this.cursor</code> is a global offset (relative to the 
document), but <code>getIndexFromOffset()</code> expects a local offset 
(relative to <code>parent</code>). I suspect your testing didn't show this 
because it so happened that <code>parent == document</code>. r103447 touches a 
lot of this code, but it looks like this issue still exists.

Speaking of testing, you should add test cases for the code you introduced.

<pre>+          console.log( parent, index );</pre>
was removed in r103447.

_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview

Reply via email to