User "Catrope" changed the status of MediaWiki.r103271. Old Status: new New Status: fixme
User "Catrope" also posted a comment on MediaWiki.r103271. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/103271#c26105 Commit summary: Rewrote prepareRemoval to support dropping nodes that are considered droppable (not tableCells) and are covered completely by the range - otherwise nodes are stripped of content Comment: <pre> + * Remove content only, and completely covered droppable nodes drop the nodes entirely. </pre> That's not a coherent sentence. <pre> + function strip( tx, node, range, offset ) { </pre> tx and offset are undocumented. <pre> + selectedNodes = node.selectNodes( range ), </pre> I don't think this code will even work if you don't enable shallow mode. I haven't proven this yet, but I'll add a test case. <pre> + tx.pushRemove( doc.data.slice( left, right ) ); </pre> I would really prefer to use the globalRange objects returned by selectNodes here. That way you don't have to loop over the children again, you probably also don't have to use shallow mode and recurse, etc., life will just be easier. <pre> + node1 && + node2 && + node1 === node2 && + range.start < range.end </pre> I don't know if you realize this, but these conditions (except start < end) are unnecessary and never really used, because if node1 and node2 are the same node, they will also have the same element type and the same parent, so the left-hand side of the or operator will be true and this bit is never evaluated. If the left-hand side failed, the right-hand side is sure to fail. As a general note, it looks like this code fails in its endeavor to ensure that nodes of unequal types are not merged. It seems strip() is expected to guard against that, but it doesn't; all it does is make sure that table cells are blanked instead of being dropped. Offhand I think that preventing nodes with unequal parents from being merged is already accomplished by using selectNodes() in strip(). I can't prove any of this, so I'll write test cases :) . _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
