[Webkit-unassigned] [Bug 68679] REGRESSION(87010): elements in ECMA-cloud neither filled nor blurred - crash on Chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68679





--- Comment #11 from Nikolas Zimmermann <zimmermann at kde.org>  2011-09-28 11:34:21 PST ---
(In reply to comment #9)
> Is there any reason we actually care about the prefix? By the time it gets to isSupportedAttribute, it's a QualifiedName, which is a triple{prefix, name, namespace}. It seems to me that we only care about the name and the namespace; something earlier than us has already taken care of mapping the prefix to the namespace.
No, we don't care for the prefix - it's just buggy. The NS matters, the prefix is user defined and not relevant for us.

> Change SVGURIReference::addSupportedAttributes to just:
> 
> >    void SVGURIReference::addSupportedAttributes(HashSet<QualifiedName>& supportedAttributes)
> >    {
> >      supportedAttributes.add(XLinkNames::hrefAttr);
> >    }

That just removes the xlink prefix, that I manually added before there, right? If so, this is the right change.

> In SVGGradientElement::isSupportedAttribute (because the element with the xlink on it in our example here is a gradient):
> 
> >    QualifiedName attrNameWithoutPrefix(attrName);
> >    attrNameWithoutPrefix.setPrefix(nullAtom);
> 
> And then supportedAttributes.contains(attrNameWithoutPrefix).
> 
> Effectively, only having the namespaced (non-prefixed) version of the attribute name in supportedAttributes, and dropping the prefix from the attribute before checking it. This fixes our bug, and I double checked to make sure that it is still matching on the namespaces (it is).
> 
> So that might be a valid fix.

I remember I had it designed this way at some point, but something wasn't working. Maybe we can override contains() in a way that it automatically matches without comparing the prefix.

For that we'd need to add some magic to supply a custom HashSet comparision function - there are existing examples how to do that, that compares QNames without checking for prefix equality.

    bool matches(const QualifiedName& other) const { return m_impl == other.m_impl || (localName() == other.localName() && namespaceURI() == other.namespaceURI()); }

The QName::matches function is specifically designed for this.
To summarize:
* Your SVGURIReference::isSupportedAttr change is correct and should be done
* We need a customizable contains() implementation, that compares QNames by calling matches().

> Is there an earlier place where we could strip the prefix instead of touching every single isSupportedAttribute? I haven't decided yet.
> 
> I don't see why we should care about the prefix at all. Just the namespace.
Right.

Hope that works?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.