[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

Attachment 38774: Patch v1

------- 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