[webkit-reviews] review granted: [Bug 25822] DOM normalize does not remove empty text node between element nodes : [Attachment 30421] Patch with more thorough set of test cases

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun May 17 08:51:14 PDT 2009


Darin Adler <darin at apple.com> has granted Kai Brüning <kai at granus.net>'s
request for review:
Bug 25822: DOM normalize does not remove empty text node between element nodes
https://bugs.webkit.org/show_bug.cgi?id=25822

Attachment 30421: Patch with more thorough set of test cases
https://bugs.webkit.org/attachment.cgi?id=30421&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   // Merge non-empty text nodes.
> +	   while (1) {
> +	       Node* nextSibling = node->nextSibling();
> +	       if (!nextSibling || !nextSibling->isTextNode()) {
> +		   node = node->traverseNextNodePostOrder();
> +		   break;
> +	       }

I think it would be clearer to have the node =
node->traverseNextNodePostOrder() line outside the if statement. I also would
prefer to have a while condition instead of just while (1).

Here's how I'd write it:

    while (Node* nextSibling = node->nextSibling()) {
	if (!nextSiblng->isTextNode())
	    break;
	...
    }

    node = node->traverseNextNodePostOrder();

Patch looks good, r=me even without that change.


More information about the webkit-reviews mailing list