13611 2007-05-07 08:48:20 -0700 Crash in setAttributeNS setting href of SVG <use> to nonexistent symbol 2007-07-16 12:19:41 -0700 1 1 1 Unclassified WebKit SVG 523.x (Safari 3) Mac OS X 10.4 RESOLVED FIXED P1 Normal --- 1 gera.weiss zimmermann oldest_to_newest 11019 0 gera.weiss 2007-05-07 08:48:20 -0700 <?xml version="1.0"?> <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" onload="init()"> <script><![CDATA[ function init() { alert("Wait") var c= document.getElementById("tt") c.setAttributeNS("http://www.w3.org/1999/xlink","href","#no") alert("Done") } ]]> </script> <use id="tt" xlink:href="#no"/> </svg> 11029 1 ap 2007-05-07 11:46:14 -0700 ASSERTION FAILED: target (/Users/ap/WebKit/WebCore/ksvg2/svg/SVGUseElement.cpp:242 virtual void WebCore::SVGUseElement::buildPendingResource()) Crashes release build. 10680 2 eric 2007-05-08 04:06:03 -0700 Ok, well this is a brute-force fix: Index: ksvg2/svg/SVGUseElement.cpp =================================================================== --- ksvg2/svg/SVGUseElement.cpp (revision 21299) +++ ksvg2/svg/SVGUseElement.cpp (working copy) @@ -142,10 +142,14 @@ if (!attached()) return; + // do a complete rebuild of the renderer if the target changed. + if (attr->name().matches(XLinkNames::hrefAttr)) { + detach(); + attach(); + } // Only update the tree if x/y/width/height or xlink:href changed. - if (attr->name() == SVGNames::xAttr || attr->name() == SVGNames::yAttr || - attr->name() == SVGNames::widthAttr || attr->name() == SVGNames::heightAttr || - attr->name().matches(XLinkNames::hrefAttr)) + else if (attr->name() == SVGNames::xAttr || attr->name() == SVGNames::yAttr || + attr->name() == SVGNames::widthAttr || attr->name() == SVGNames::heightAttr) buildPendingResource(); else if (m_shadowTreeRootElement) m_shadowTreeRootElement->setChanged(); 10681 3 14409 eric 2007-05-08 04:17:31 -0700 Created attachment 14409 Better test case This test case tests both the crash, as well as testing to make sure that the id gets added to the pending list if appropriate. (My above simple fix does not handle that case properly). This example shows how fragile the current pending list system is... but that's a subject for another bug. 10682 4 eric 2007-05-08 04:19:58 -0700 Actually, the pending list issue may need to be split out. Even if "#green" is part of the initial source text, WebKit still doesn't notice when "wrongname" changes to "green" and properly update the document. 8624 5 eric 2007-05-31 13:52:57 -0700 A slightly more complete brute-force fix: Index: ksvg2/svg/SVGUseElement.cpp =================================================================== --- ksvg2/svg/SVGUseElement.cpp (revision 21912) +++ ksvg2/svg/SVGUseElement.cpp (working copy) @@ -142,10 +142,15 @@ if (!attached()) return; - // Only update the tree if x/y/width/height or xlink:href changed. - if (attr->name() == SVGNames::xAttr || attr->name() == SVGNames::yAttr || - attr->name() == SVGNames::widthAttr || attr->name() == SVGNames::heightAttr || - attr->name().matches(XLinkNames::hrefAttr)) + if (attr->name().matches(XLinkNames::hrefAttr)) { + // if the target changed recreate the entire rendering sub-tree + detach(); + m_targetElementInstance = 0; + m_shadowTreeRootElement = 0; + attach(); + } else if (attr->name() == SVGNames::xAttr || attr->name() == SVGNames::yAttr || + attr->name() == SVGNames::widthAttr || attr->name() == SVGNames::heightAttr) + // otherwise only update the tree if x/y/width/height changed. buildPendingResource(); else if (m_shadowTreeRootElement) m_shadowTreeRootElement->setChanged(); 7920 6 zimmermann 2007-06-11 17:44:31 -0700 Investigate to find a real fix for the xlink:href problems, which don't involve detach/attach. 4635 7 zimmermann 2007-07-16 06:34:35 -0700 This doesn't crash anymore in feature-branch. Though it just silently ignores the request, instead of cleaning up the use element, to not show anything anymore. About to fix that. 4606 8 zimmermann 2007-07-16 12:19:41 -0700 Landed in r24320. 14409 2007-05-08 04:17:31 -0700 2007-05-08 04:17:31 -0700 Better test case test.svg image/svg+xml 781 eric PD94bWwgdmVyc2lvbj0iMS4wIj8+CjxzdmcgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAv c3ZnIgogeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIG9ubG9hZD0i aW5pdCgpIj4KIDxzY3JpcHQ+CiAgIDwhW0NEQVRBWwogICAgdmFyIHhsaW5rTnMgPSAiaHR0cDov L3d3dy53My5vcmcvMTk5OS94bGluayI7CiAgICAgIGZ1bmN0aW9uIGluaXQoKSB7CiAgICAgICAg dmFyIGMgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgidHQiKTsKICAgIAogICAgICAgIC8vIFRo aXMgbGluZSBjaGVja3MgZm9yIGEgY3Jhc2ggKDEzNjExKQogICAgICAgIGMuc2V0QXR0cmlidXRl TlMoeGxpbmtOcywiaHJlZiIsIiNncmVlbiIpOwogICAgCiAgICAgICAgLy8gVGhpcyBsaW5lIG1h a2VzIHN1cmUgdGhhdCB0aGUgcmVmZXJlbmNlIHdhcyBwcm9wZXJseSBhZGRlZCB0byB0aGUgcGVu ZGluZyBsaXN0CiAgICAgICAgZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoIndyb25nbmFtZSIpLmlk ID0gImdyZWVuIjsKICAgICAgfQogICAgXV0+CiAgPC9zY3JpcHQ+CgogIDxkZWZzPgogICAgPHN5 bWJvbCBpZD0id3JvbmduYW1lIj4KICAgICAgPHJlY3Qgd2lkdGg9IjEwMCIgaGVpZ2h0PSIxMDAi IGZpbGw9ImdyZWVuIiAvPgogICAgPC9zeW1ib2w+CiAgPC9kZWZzPgoKICA8cmVjdCB3aWR0aD0i MTAwIiBoZWlnaHQ9IjEwMCIgZmlsbD0icmVkIiAvPgogIDx1c2UgaWQ9InR0IiB4bGluazpocmVm PSIjbm8iIHdpZHRoPSIxMDAiIGhlaWdodD0iMTAwIi8+Cjwvc3ZnPg==