[webkit-reviews] review denied: [Bug 46487] Implement CSSStyleRule::setSelectorText() : [Attachment 68722] [PATCH] Suggested solution

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 46487: Implement CSSStyleRule::setSelectorText()
https://bugs.webkit.org/show_bug.cgi?id=46487

Attachment 68722: [PATCH] Suggested solution
https://bugs.webkit.org/attachment.cgi?id=68722&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68722&action=review

Can you add some tests of invalid selectors?

> WebCore/css/CSSStyleRule.cpp:57
> +void CSSStyleRule::setSelectorText(const String& selectorText,
ExceptionCode& /*ec*/)

We should remove the ExcpectionCode parameter (and the raises in the IDL file)
since the latest spec
(http://dev.w3.org/csswg/cssom/#dom-cssstylerule-selectortext) doesn't have it
ever raising.

> WebCore/css/CSSStyleRule.cpp:60
> +    CSSParser p;
> +    CSSSelectorList selectorList;

There is no reason to declare these at the top of the function.