Reviewed by Darin & Eric.

Fixes: http://bugs.webkit.org/show_bug.cgi?id=17258 (SVG uses erroneous cursor implementation)

SVG cursors are not well-integrated within the CSS(3) cursor support in WebCore.
SVGCursorElement duplicates CSSCursorImageValue functionality and inherits from
CachedResourceClient itself, handling remote-image acquisation on its own.

RenderStyle's CursorData class holds "IntPoint hotSpot", "CachedImage* image"
and just for SVG a 'String cursorFragmentId' (a reference to a SVG <cursor> element, by id).

SVG stores a reference to a SVGCursorElement, which holds a CachedImage pointer itself -
instead of storing the CachedImage in the CursorData class, as it's supposed to be.
Because of that several places in WebCore contain special SVG cursor handling - which
is unneeded.

Fix all issues by integrating within CSSCursorImageValue, remove 'String cursorFragmentId'
from RenderStyle, kill any special SVG cursor handling in WebCore and fix dynamic attribute
changes through DOM / SVG DOM (scripting of 'x' / 'y' / 'xlink:href' attribute).


Canonical link: https://commits.webkit.org/24016@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@30208 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog

@@ -1,3 +1,58 @@
+2008-02-13  Nikolas Zimmermann  <zimmermann@kde.org>
+
+        Reviewed by Darin & Eric.
+
+        Fixes: http://bugs.webkit.org/show_bug.cgi?id=17258 (SVG uses erroneous cursor implementation)
+
+        SVG cursors are not well-integrated within the CSS(3) cursor support in WebCore.
+        SVGCursorElement duplicates CSSCursorImageValue functionality and inherits from
+        CachedResourceClient itself, handling remote-image acquisation on its own.
+
+        RenderStyle's CursorData class holds "IntPoint hotSpot", "CachedImage* image"
+        and just for SVG a 'String cursorFragmentId' (a reference to a SVG <cursor> element, by id).
+
+        SVG stores a reference to a SVGCursorElement, which holds a CachedImage pointer itself -
+        instead of storing the CachedImage in the CursorData class, as it's supposed to be.
+        Because of that several places in WebCore contain special SVG cursor handling - which
+        is unneeded.
+
+        Fix all issues by integrating within CSSCursorImageValue, remove 'String cursorFragmentId'
+        from RenderStyle, kill any special SVG cursor handling in WebCore and fix dynamic attribute
+        changes through DOM / SVG DOM (scripting of 'x' / 'y' / 'xlink:href' attribute). Now you
+        can script the mouse cursor location using SVG - the feature anyone has waited for.
+
+        Added manual test case: manual-tests/svg-cursor-changes.svg (no support for cursors in DRT)
+
+        * css/CSSCursorImageValue.cpp:
+        (WebCore::isSVGCursorIdentifier):
+        (WebCore::resourceReferencedByCursorElement):
+        (WebCore::CSSCursorImageValue::~CSSCursorImageValue):
+        (WebCore::CSSCursorImageValue::updateIfNeeded):
+        (WebCore::CSSCursorImageValue::image):
+        * css/CSSCursorImageValue.h:
+        * css/CSSImageValue.cpp:
+        (WebCore::CSSImageValue::image):
+        * css/CSSImageValue.h:
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseValue):
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::applyProperty):
+        * manual-tests/svg-cursor-changes.svg: Added.
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::selectCursor):
+        * rendering/RenderStyle.cpp:
+        * rendering/RenderStyle.h:
+        (WebCore::CursorData::operator==):
+        * svg/SVGCursorElement.cpp:
+        (WebCore::SVGCursorElement::SVGCursorElement):
+        (WebCore::SVGCursorElement::~SVGCursorElement):
+        (WebCore::SVGCursorElement::parseMappedAttribute):
+        (WebCore::SVGCursorElement::addClient):
+        (WebCore::SVGCursorElement::removeClient):
+        (WebCore::SVGCursorElement::svgAttributeChanged):
+        * svg/SVGCursorElement.h:
+        (WebCore::SVGCursorElement::isValid):
+
 2008-02-13  Alp Toker  <alp@atoker.com>
 
         Reviewed by Adam Roben.

WebCore/css/CSSCursorImageValue.cpp

@@ -1,7 +1,8 @@
 /**
  * This file is part of the DOM implementation for KDE.
  *
- * Copyright (C) 2006, Rob Buis <buis@kde.org>
+ * Copyright (C) 2006 Rob Buis <buis@kde.org>
+ *           (C) 2008 Nikolas Zimmermann <zimmermann@kde.org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -22,16 +23,101 @@
 #include "config.h"
 #include "CSSCursorImageValue.h"
 
+#if ENABLE(SVG)
+#include "CachedImage.h"
+#include "DocLoader.h"
+#include "PlatformString.h"
+#include "SVGCursorElement.h"
+#include "SVGURIReference.h"
+#endif
+
 namespace WebCore {
 
+#if ENABLE(SVG)
+inline bool isSVGCursorIdentifier(const String& url)
+{
+    KURL kurl(url.deprecatedString());
+    return kurl.hasRef();
+}
+
+inline SVGCursorElement* resourceReferencedByCursorElement(const String& fragmentId, Document* document)
+{
+    Element* element = document->getElementById(SVGURIReference::getTarget(fragmentId));
+    if (element && element->hasTagName(SVGNames::cursorTag))
+        return static_cast<SVGCursorElement*>(element);
+
+    return 0;
+}
+#endif
+
 CSSCursorImageValue::CSSCursorImageValue(const String& url, const IntPoint& hotspot, StyleBase* style)
     : CSSImageValue(url, style)
     , m_hotspot(hotspot)
+    , m_referencedElement(0)
 {
 }
 
 CSSCursorImageValue::~CSSCursorImageValue()
 {
+#if ENABLE(SVG)
+    const String& url = getStringValue();
+    if (m_referencedElement && m_referencedElement->document() && isSVGCursorIdentifier(url)) {
+        if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(url, m_referencedElement->document()))
+            cursorElement->removeClient(m_referencedElement);
+    }
+#endif
+}
+
+bool CSSCursorImageValue::updateIfSVGCursorIsUsed(Element* element)
+{
+#if ENABLE(SVG)
+    if (!element || !element->isSVGElement())
+        return false;
+
+    const String& url = getStringValue();
+    if (!isSVGCursorIdentifier(url))
+        return false;
+
+    if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(url, element->document())) {
+        int x = roundf(cursorElement->x().value());
+        if (x != m_hotspot.x())
+            m_hotspot.setX(x);
+
+        int y = roundf(cursorElement->y().value());
+        if (y != m_hotspot.y())
+            m_hotspot.setY(y);
+
+        if (m_image && m_image->url() != element->document()->completeURL(cursorElement->href().deprecatedString())) {
+            m_image->deref(this);
+            m_image = 0;
+
+            m_accessedImage = false;
+        }
+
+        if (m_referencedElement != element)
+            cursorElement->removeClient(m_referencedElement);
+
+        m_referencedElement = static_cast<SVGElement*>(element);
+        cursorElement->addClient(m_referencedElement);
+        return true;
+    }
+#endif
+
+    return false;
+}
+
+CachedImage* CSSCursorImageValue::image(DocLoader* loader)
+{
+    String url = getStringValue();
+
+#if ENABLE(SVG) 
+    if (isSVGCursorIdentifier(url) && loader && loader->doc()) {
+        if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(url, loader->doc()))
+            url = cursorElement->href();
+    }
+#endif
+
+    return CSSImageValue::image(loader, url);
 }
 
 } // namespace WebCore

WebCore/css/CSSCursorImageValue.h

@@ -1,7 +1,7 @@
 /*
  * This file is part of the DOM implementation for KDE.
  *
- * Copyright (C) 2006, Rob Buis <buis@kde.org>
+ * Copyright (C) 2006 Rob Buis <buis@kde.org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -27,15 +27,25 @@
 
 namespace WebCore {
 
+class Element;
+class SVGElement;
+
 class CSSCursorImageValue : public CSSImageValue {
 public:
     CSSCursorImageValue(const String& url, const IntPoint& hotspot, StyleBase*);
     virtual ~CSSCursorImageValue();
 
     IntPoint hotspot() const { return m_hotspot; }
 
-protected:
+    bool updateIfSVGCursorIsUsed(Element*);
+    virtual CachedImage* image(DocLoader*);
+
+private:
     IntPoint m_hotspot;
+
+#if ENABLE(SVG)
+    SVGElement* m_referencedElement;
+#endif
 };
 
 } // namespace WebCore

WebCore/css/CSSImageValue.cpp

@@ -51,15 +51,20 @@ CSSImageValue::~CSSImageValue()
 }
 
 CachedImage* CSSImageValue::image(DocLoader* loader)
+{
+    return image(loader, getStringValue());
+}
+
+CachedImage* CSSImageValue::image(DocLoader* loader, const String& url)
 {
     if (!m_accessedImage) {
         m_accessedImage = true;
 
         if (loader)
-            m_image = loader->requestImage(getStringValue());
+            m_image = loader->requestImage(url);
         else
             // FIXME: Should find a way to make these images sit in their own memory partition, since they are user agent images.
-            m_image = static_cast<CachedImage*>(cache()->requestResource(0, CachedResource::ImageResource, KURL(getStringValue().deprecatedString()), 0, 0));
+            m_image = static_cast<CachedImage*>(cache()->requestResource(0, CachedResource::ImageResource, KURL(url.deprecatedString()), 0, 0));
 
         if (m_image)
             m_image->ref(this);

WebCore/css/CSSImageValue.h

@@ -30,15 +30,18 @@ namespace WebCore {
 
 class DocLoader;
 
-class CSSImageValue : public CSSPrimitiveValue, public CachedResourceClient {
+class CSSImageValue : public CSSPrimitiveValue,
+                      public CachedResourceClient {
 public:
     CSSImageValue();
     CSSImageValue(const String& url, StyleBase*);
     virtual ~CSSImageValue();
 
-    CachedImage* image(DocLoader*);
+    virtual CachedImage* image(DocLoader*);
 
 protected:
+    CachedImage* image(DocLoader*, const String& url);
+
     CachedImage* m_image;
     bool m_accessedImage;
 };

WebCore/css/CSSParser.cpp

@@ -815,11 +815,6 @@ bool CSSParser::parseValue(int propId, bool important)
             } else if(strict && nrcoords == 2)
                 hotspot = IntPoint(coords[0], coords[1]);
             if (strict || coords.size() == 0) {
-#if ENABLE(SVG)
-                if (uri.startsWith("#"))
-                    list->append(new CSSPrimitiveValue(uri, CSSPrimitiveValue::CSS_URI));
-                else
-#endif
                 if (!uri.isEmpty()) {
                     list->append(new CSSCursorImageValue(
                                  KURL(styleElement->baseURL().deprecatedString(), uri.deprecatedString()).string(),

WebCore/css/CSSStyleSelector.cpp

@@ -2773,15 +2773,10 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
                 primitiveValue = static_cast<CSSPrimitiveValue*>(item);
                 int type = primitiveValue->primitiveType();
                 if (type == CSSPrimitiveValue::CSS_URI) {
-#if ENABLE(SVG)
-                    if (primitiveValue->getStringValue().find("#") == 0)
-                        m_style->addSVGCursor(primitiveValue->getStringValue().substring(1));
-                    else
-#endif
-                    {
-                        CSSCursorImageValue* image = static_cast<CSSCursorImageValue*>(primitiveValue);
-                        m_style->addCursor(image->image(m_element->document()->docLoader()), image->hotspot());
-                    }
+                    CSSCursorImageValue* image = static_cast<CSSCursorImageValue*>(primitiveValue);
+                    if (image->updateIfSVGCursorIsUsed(m_element)) // Elements with SVG cursors are not allowed to share style.
+                        m_style->setUnique();
+                    m_style->addCursor(image->image(m_element->document()->docLoader()), image->hotspot());
                 } else if (type == CSSPrimitiveValue::CSS_IDENT)
                     m_style->setCursor(*primitiveValue);
             }

WebCore/page/EventHandler.cpp

@@ -60,9 +60,7 @@
 #include "TextEvent.h"
 
 #if ENABLE(SVG)
-#include "SVGCursorElement.h"
 #include "SVGDocument.h"
-#include "SVGLength.h"
 #include "SVGNames.h"
 #endif
 
@@ -696,16 +694,6 @@ Cursor EventHandler::selectCursor(const MouseEventWithHitTestResults& event, Pla
         for (unsigned i = 0; i < cursors->size(); ++i) {
             CachedImage* cimage = (*cursors)[i].cursorImage;
             IntPoint hotSpot = (*cursors)[i].hotSpot;
-#if ENABLE(SVG)
-            if (!cimage) {
-                Element* e = node->document()->getElementById((*cursors)[i].cursorFragmentId);
-                if (e && e->hasTagName(cursorTag)) {
-                    hotSpot.setX(int(static_cast<SVGCursorElement*>(e)->x().value()));
-                    hotSpot.setY(int(static_cast<SVGCursorElement*>(e)->y().value()));
-                    cimage = static_cast<SVGCursorElement*>(e)->cachedImage();
-                }
-            }
-#endif
             if (!cimage)
                 continue;
             if (cimage->image()->isNull())

WebCore/rendering/RenderStyle.cpp

@@ -1354,15 +1354,6 @@ void RenderStyle::addCursor(CachedImage* image, const IntPoint& hotSpot)
     inherited.access()->cursorData->append(data);
 }
 
-void RenderStyle::addSVGCursor(const String& fragmentId)
-{
-    CursorData data;
-    data.cursorFragmentId = fragmentId;
-    if (!inherited.access()->cursorData)
-        inherited.access()->cursorData = new CursorList;
-    inherited.access()->cursorData->append(data);
-}
-
 void RenderStyle::setCursorList(PassRefPtr<CursorList> other)
 {
     inherited.access()->cursorData = other;

WebCore/rendering/RenderStyle.h

@@ -1284,13 +1284,12 @@ struct CursorData {
     {}
 
     bool operator==(const CursorData& o) const {
-        return hotSpot == o.hotSpot && cursorImage == o.cursorImage && cursorFragmentId == o.cursorFragmentId;
+        return hotSpot == o.hotSpot && cursorImage == o.cursorImage;
     }
     bool operator!=(const CursorData& o) const { return !(*this == o); }
 
     IntPoint hotSpot; // for CSS3 support
     CachedImage* cursorImage; // weak pointer, the CSSValueImage takes care of deleting cursorImage
-    String cursorFragmentId; // only used for SVGCursorElement, a direct pointer would get stale
 };
 
 class CursorList : public RefCounted<CursorList> {
@@ -1983,7 +1982,6 @@ class RenderStyle {
 
     void setCursor( ECursor c ) { inherited_flags._cursor_style = c; }
     void addCursor(CachedImage*, const IntPoint& = IntPoint());
-    void addSVGCursor(const String&);
     void setCursorList(PassRefPtr<CursorList>);
     void clearCursorList();