[webkit-reviews] review granted: [Bug 6002] WebKit does not properly handle SVG <cursor> element : [Attachment 10701] css parser fixes

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Fri Sep 22 03:03:52 PDT 2006

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

Attachment 10701: css parser fixes

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
I don't think we want this:
+	     if (uri.startsWith("#")) {
+		 if (!list)
+		     list = new CSSValueList; 
+		 list->append(new CSSPrimitiveValue(uri,
+		 value = valueList->next();
+	     } else

That treats url(#foo) differently from url(foo.png).  The latter allows CSS3
hotspots, the former does not.	I'm not sure which should win out (the <cursor>
specified hotspot or the css3  one, but I don't think it should be an error.)

I'm surprised this is only an error in strict mode:
+		 if (strict && value && !(value->unit == Value::Operator &&
value->iValue == ','))
+		     return false;

If it's only going to be an error in strict mode, maybe it only needs to be
required in strict mode?  Or maybe in non-strict mode an ; is also allowed?

I think this function should have a different name to denote its different
perhaps addSVGCursor ?

You might also want to call the parameter fragmentId:
+void RenderStyle::addCursor(const String& id)

Great patch.  This has been an awfully long journey to get this landed, but I
think you're basically there.  I don't need to see the code again.   But if you
could make those tweaks before landing that would be great.  I still need to
check the test case.


More information about the webkit-reviews mailing list