[webkit-reviews] review granted: [Bug 25822] DOM normalize does not remove empty text node between element nodes : [Attachment 30390] Patch to fix this bug

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 15 10:26:11 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 30390: Patch to fix this bug
https://bugs.webkit.org/attachment.cgi?id=30390&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -    for (; node; node = node->traverseNextNodePostOrder()) {
> +    for (; node; ) {

Should be changed a while, then.

> +	   } else {
> +	       node = node->traverseNextNodePostOrder();

WebKit style doesn't use braces around single lines. Typically we'd use "early
continue" in a case like this:

    if (type != TEXT_NODE) {
	node = node->traverseNextNodePostOrder();
	continue;
    }

I'd really like to see a more thorough set of test cases. Adding a test for
only the single case that shows the current bug seems too minimal.

r=me as is -- none of my requested changes are mandatory


More information about the webkit-reviews mailing list