[webkit-reviews] review denied: [Bug 28828] Make SVGStyleElement title/media/type setters do something : [Attachment 38774] Patch v1
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Aug 31 03:17:14 PDT 2009
Eric Seidel <eric at webkit.org> has denied Cameron McCormack <cam at mcc.id.au>'s
request for review:
Bug 28828: Make SVGStyleElement title/media/type setters do something
https://bugs.webkit.org/show_bug.cgi?id=28828
Attachment 38774: Patch v1
https://bugs.webkit.org/attachment.cgi?id=38774&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
shouldBeEqualToString will make your life easier here:
5 shouldBe("style.type", "'text/css'");
shouldBeEqualToString("style.type", "text/css");
Normally we would use longer argument names here:
53 void SVGStyleElement::setXmlspace(const AtomicString& s, ExceptionCode&)
like "space"
and "type":
65 void SVGStyleElement::setType(const AtomicString& s)
Why not just remove "setter raises()"?
33 /*setter raises(DOMException)*/;
Are these spec'd to raise, but they never should? If so, please explain in
your ChangeLog.
In general this looks great! If you were a committer, I would r+ and you could
land with changes.
More information about the webkit-reviews
mailing list