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

[bugzilla-daemon at opendarwin.org]

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





------- Comment #7 from macdome at opendarwin.org  2006-05-23 13:39 PDT -------
(From update of attachment 8494)
+        } else if (value)
+            return value;
         ASSERT_NOT_REACHED();

this would be clearer as
ASSERT(value);
return value;

style issues:
+        CSSValueList* list = 0;
+        CSSValue *value = 0;

This shoudl be broken up into multiple lines to be more readable:
+                    list->append(new
CSSCursorImageValue(String(KURL(styleElement->baseURL().deprecatedString(),
url.deprecatedString()).url()), IntPoint(x, y), styleElement));

no {} needed here:
+            } else if (!strict && value->id == CSS_VAL_HAND) { // MSIE 5
compatibility :/
+                list->append(new CSSPrimitiveValue(CSS_VAL_POINTER));
+            } else if (value->id >= CSS_VAL_AUTO && value->id <= CSS_VAL_HELP)

style:
+            for (int i = 0; i < len; i++)
+            {

we don't generally commit commented out code:
+                    //if (!style->cursors()) // load first
+                    //    image->image(element->document()->docLoader());

more style:
+                } else if (type == CSSPrimitiveValue::CSS_IDENT) {
+                    style->setCursor((ECursor)(primitiveValue->getIdent() -
CSS_VAL_AUTO));
+                }

more style:
+        if (SVGURIReference::parseMappedAttribute(attr)) return;

(return should be on a separate line)

style:
+            if (!cimage) continue;

this shoudl probably have an argument label to indicate that the IntPoint is a
hotspot:

+        Cursor(Image*, IntPoint = IntPoint());

same here, ideally p should be named "hotspot" or something more descriptive:
+static NSCursor* createCustomCursor(Image* image, IntPoint p)

These don't need argument labels, as they provide no value here:
+    void setCursors(CursorInfo* o);
+    void addCursor(CSSCursorImageValue* v);

more style:
+: m_cursor(cursor), m_next(0)

+    if (m_cursor) m_cursor->ref();

Mostly I ran through style issues in my review.


-- 
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.