[Webkit-unassigned] [Bug 6002] WebKit does not properly handle SVG <cursor> element

bugzilla-daemon at opendarwin.org bugzilla-daemon at opendarwin.org
Sun Sep 10 19:56:36 PDT 2006


http://bugzilla.opendarwin.org/show_bug.cgi?id=6002


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #10491|review?                     |review-
               Flag|                            |




------- Comment #25 from darin at apple.com  2006-09-10 19:56 PDT -------
(From update of attachment 10491)
Some nitpicks:

+            for (unsigned i = 0;i < cursors->size();++i)

Missing spaces after the semicolons.

+        if (list) {
+            list->append(value);
+            return list;
+        }
+        ASSERT(value);

The ASSERT(value) should go before the if.

+                    if (primitiveValue->getStringValue().find("#") == 0)

You fixed one of these, but not the other!

+        Cursor(Image*, IntPoint hotspot);

Should be const IntPoint& instead of IntPoint, and in fact some of the
implementations, like the CursorQt.cpp ones, are like that.

+        inherited.access()->cursorData = new CursorList();

It's slightly nicer to just do "new CursorList" in a case like this, I think.

+class CursorList : public Shared<CursorList>
+{

Brace should be on the same line as the class declaration.

+    void setCursors(CursorList*);

This should take a PassRefPtr<CursorList> since it takes ownership of the
passed-in object.

+    CachedImage* cursorImage; // weak pointer, the CSSValueImage takes care of
deleting cursorImage

I'm a little concerned about this. What's the guarantee that the RenderStyle
won't outlast the CSSValueImage? Is there any other RenderStyle code that uses
this ownership model?

I'm going to say review- because of the find() and because of my previous
comment about what I called the "CSS_VAL_AUTO case".


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list