[webkit-reviews] review granted: [Bug 47971] css combinator "+" in combination with NAV tag is buggy : [Attachment 98129] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Chiculita Alexandru
<achicu at adobe.com>'s request for review:
Bug 47971: css combinator "+" in combination with NAV tag is buggy
https://bugs.webkit.org/show_bug.cgi?id=47971

Attachment 98129: Patch
https://bugs.webkit.org/attachment.cgi?id=98129&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98129&action=review

Patch looks good. Please consider addressing my two comments before committing
(or if you're not a committer, you can post a new patch and I can cq+ it for
you).

> LayoutTests/fast/css/div_plus_nav_bug47971.html:6
> +#one + nav { color: green; }
> +#two+nav { color: green; }
> +label+nav { color: green; }

For thoroughness sake, maybe add the following cases as well:
#foo +nav
#bar+ nav

> LayoutTests/fast/css/div_plus_nav_bug47971.html:42
> +    } catch(e) {
> +	   document.writeln("Fail: " + element + ": Exception \"" + e + "\".<br
/>");
> +    }

When would you ever hit this exception? Do you need the try/catch at all? It's
fine for the test to throw an error if there is a JS error. It's worth having
less extra cruft in the test.