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

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
http://bugzilla.opendarwin.org/show_bug.cgi?id=6002

Attachment 10701: css parser fixes
http://bugzilla.opendarwin.org/attachment.cgi?id=10701&action=edit

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

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
behavior:
style->addCursor(primitiveValue->getStringValue().substring(1))
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.

r=me.