[webkit-reviews] review denied: [Bug 28303] [Qt] XSLT support with QtXmlPatterns : [Attachment 39427] [Qt] Implement XSLT support with QtXmlPatterns.

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Jakub Wieczorek
<faw217 at gmail.com>'s request for review:
Bug 28303: [Qt] XSLT support with QtXmlPatterns
https://bugs.webkit.org/show_bug.cgi?id=28303

Attachment 39427: [Qt] Implement XSLT support with QtXmlPatterns.
https://bugs.webkit.org/attachment.cgi?id=39427&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>

>From what I can see this is very good, except for a tiny TransformSource
buglet:

> -void Document::setTransformSource(void* doc)
> +void Document::setTransformSource(TransformSource* source)
>  {
> -    if (doc == m_transformSource)
> -	   return;
> -
> -    xmlFreeDoc((xmlDocPtr)m_transformSource);
> -    m_transformSource = doc;
> +    m_transformSource = source;
>  }

Technically this function was protected against self-assignment and isn't
anymore. One could
argue it's a bug in OwnPtr perhaps?

> +    void setTransformSource(TransformSource*);
> +    TransformSource* transformSource() const { return
m_transformSource.get(); }
>  #endif

I believe the setter should take a PassOwnPtr<TransformSource>, to make it safe
and clear that ownership is passed here.