[webkit-reviews] review denied: [Bug 6002] WebKit does not properly handle SVG <cursor> element : [Attachment 10483] Another go

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sun Sep 10 01:45:51 PDT 2006

Eric Seidel <macdome at opendarwin.org> has denied Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 6002: WebKit does not properly handle SVG <cursor> element

Attachment 10483: Another go

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
Storing a Vector on RenderStyle is not a good idea.  That uses at least 4 more
bytes than it needs to.  Storing a pointer to a vector would be OK, or using a
list, both would work (personally I'm partial to the list).

Creating a CSSValueList unconditionally, and then deleting it if not needed is

+	 CSSValueList* list = new CSSValueList;

A better method is to have an if in your while statement which checks
if (!list)
    list = new CSSValueList;

Copy-paste error, you set the X value of the hotspot twice:
+		     hotspot.setX(value->fValue);

Also, you don't properly error out when it's
cursor: url("foo.png") 1, auto;
a single number should be an error.

I still would argue that a missing last value should only be allowed in quirks

+	     if (!value) { // no value after url list (MSIE 5 compatibility)


This doesn't need to use a deprecatedString.  .substring(1) will do the same as

Otherwise it looks fine.
Need one more round on this.

More information about the webkit-reviews mailing list