[webkit-reviews] review denied: [Bug 31875] QWebView: Impossible to make XMLHttpRequest from locally stored HTML page : [Attachment 45223] Proposed Patch; Addressed Laszlo's concerns

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Carol Szabo
<carol.szabo at nokia.com>'s request for review:
Bug 31875: QWebView: Impossible to make XMLHttpRequest from locally stored HTML
page
https://bugs.webkit.org/show_bug.cgi?id=31875

Attachment 45223: Proposed Patch; Addressed Laszlo's concerns
https://bugs.webkit.org/attachment.cgi?id=45223&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
>  /*!
> +    \since 4.7
> +    Constructs a security origin for \a url.
> +*/
> +QWebSecurityOrigin* QWebSecurityOrigin::create(const QUrl& url)
> +{
> +    return new QWebSecurityOrigin(new
QWebSecurityOriginPrivate(SecurityOrigin::createFromString(url.toString()).get(
)));
> +};

QWebSecurityOrigin is a class that is designed to be value based, so returning
a pointer seems wrong to me. In addition
the "create" pattern is not typically used in Qt APIs.

> +*/
> +void QWebSecurityOrigin::addToWhiteList(const QUrl& url, bool
includeSubDomains)
> +{
> +    SecurityOrigin::whiteListAccessFromOrigin(*d->origin.get(),
url.scheme(), url.host(), includeSubDomains);
> +}

I think the boolean as second argument is not a good choice for the API, as it
makes the caller code hard to read.

See also

    http://doc.trolltech.com/qq/qq13-apis.html#thebooleanparametertrap

for a more detailed explanation of this pattern.

I think we sohuld discuss this API on the mailing list first ( see
https://trac.webkit.org/wiki/QtWebKitAPI )