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

bugzilla-daemon at opendarwin.org bugzilla-daemon at opendarwin.org
Sun May 21 05:00:40 PDT 2006


ap at nypop.com changed:

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

------- Comment #5 from ap at nypop.com  2006-05-21 05:00 PDT -------
(From update of attachment 8441)
I think I have enough comments to make a first round of a review :)

+        if (style->cursors())
+            return new

In Firefox, the computed style includes the whole list, not just one URI (I
tested with a bookmarklet from

+class CSSCursorImageValue : public CSSImageValue

We usually have one class per file.

+        //  [<uri>,]* [ auto | crosshair | default | pointer | progress | move
| e-resize | ne-resize |
+            valueList->next(); // comma
+            value = valueList->next();

It's probably OK not to support CSS3 hotspots at first, but here, the parser
would just get confused by them, which isn't good at all. Also, is it possible
to move the parsing logic to CSSGrammar.y?

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

Nitpick: I think custom cursors were implemented in MSIE 6.

+struct CursorInfo {
+    ~CursorInfo();
+    void clearCursor();
+    CachedImage *cursor_image;
+    IntPoint cursor_hotspot;
+    struct CursorInfo *next;

Special-casing SVG doesn't seem necessary for dealing with the hotspot (even if
we don't support CSS3 hotspots, the parser can just pass {0,0}).

-      cursor_image( o.cursor_image ), font( o.font ),
+      cursor_info( o.cursor_info ), font( o.font ),

StyleInheritedData didn't own cursor_image, but it owns cursor_info, so just
copying a pointer isn't enough.

+void RenderStyle::addCursor( CachedImage *v )

Style violation - we don't use extra spaces in braces (same issue elsewhere).

All fallback cursors are being loaded and decoded here, even if only the first
one gets used. I am not sure if this can be considered acceptable, but it's
definitely not good.

Of course, this change needs extensive test cases.

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