[Webkit-unassigned] [Bug 26371] SVGLangSpace duplicates xml:lang and xml:space values

[bugzilla-daemon at webkit.org]

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


zimmermann at kde.org changed:

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




------- Comment #2 from zimmermann at kde.org  2009-06-18 10:10 PDT -------
(From update of attachment 31239)
Hi Rob,

nice patch, going in the right direction :-)

> -void SVGElement::setXmlbase(const String& value, ExceptionCode&)
> +void SVGElement::setXmlbase(const String&, ExceptionCode& ec)
>  {
> -    setAttribute(XMLNames::baseAttr, value);
> +    //setAttribute(XMLNames::baseAttr, value);
> +    ec = NO_MODIFICATION_ALLOWED_ERR;
>  }
Is that change related? It contains commented code, hence I'm wondering.
I guess you wanted to fix "attribute [ConvertNullToNullString] DOMString
xmlbase setter raises(DOMException);"

I'd rather commit this in a seperated patch with an associated testcase.

> -void SVGLangSpace::setXmllang(const AtomicString& xmlLang)
> +void SVGLangSpace::setXmllang(const AtomicString&/*, ExceptionCode &ec*/)
>  {
> -    m_lang = xmlLang;
> +    //ec = NO_MODIFICATION_ALLOWED_ERR;
>  }
This either needs to throw an exception (then we need to correct the
SVGLangSpace.idl file as well, fixing the commented out "setter raises..."
lines).
I guess this is what W3C says IIRC. If you go for that, we need a testcase. If
you're on this anyway, I think it would make sense to go for that...

>  const AtomicString& SVGLangSpace::xmlspace() const
>  {
> -    if (!m_space) {
> -        DEFINE_STATIC_LOCAL(const AtomicString, defaultString, ("default"));
> -        return defaultString;
> -    }
> -
> -    return m_space;
> -}
> -
> -void SVGLangSpace::setXmlspace(const AtomicString& xmlSpace)
> -{
> -    m_space = xmlSpace;
> +    return contextElement()->getAttribute(XMLNames::spaceAttr);
>  }
>  
> -bool SVGLangSpace::parseMappedAttribute(MappedAttribute* attr)
> +void SVGLangSpace::setXmlspace(const AtomicString&/*, ExceptionCode& ec*/)
>  {
> -    if (attr->name().matches(XMLNames::langAttr)) {
> -        setXmllang(attr->value());
> -        return true;
> -    } else if (attr->name().matches(XMLNames::spaceAttr)) {
> -        setXmlspace(attr->value());
> -        return true;
> -    }
> -
> -    return false;
> + //   ec = NO_MODIFICATION_ALLOWED_ERR;
>  }
Same comment as above.

Everything else looks fine. Because of lacking testcases setting r- to clear it
out the review queue for now.


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