[webkit-reviews] review denied: [Bug 37880] [Qt] Patch for checking content types supported by the QWebPage... : [Attachment 65320] Function for checking content-type supported by QWebPage II...

[bugzilla-daemon at webkit.org]

Antonio Gomes <tonikitoo at webkit.org> has denied Dawit A. <adawit at kde.org>'s
request for review:
Bug 37880: [Qt] Patch for checking content types supported by the QWebPage...
https://bugs.webkit.org/show_bug.cgi?id=37880

Attachment 65320: Function for checking content-type supported by QWebPage
II...
https://bugs.webkit.org/attachment.cgi?id=65320&action=review

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
Almost there! One minor change I would like to see yet. See it below.


> +/*!
> + *  \since 4.8
> + *
> + *  Returns the list of all content types supported by QWebPage.
> + */
> +QStringList QWebPage::supportedContentTypes() const
> +{
> +    QStringList mimeTypes;
> +
> +    WebCore::Frame *frame =
d->page->focusController()->focusedOrMainFrame();
> +   
extractContentTypeFromHash(MIMETypeRegistry::getSupportedImageMIMETypes(),
&mimeTypes);
> +   
extractContentTypeFromHash(MIMETypeRegistry::getSupportedNonImageMIMETypes(),
&mimeTypes);
> +    if (frame && frame->settings() &&
frame->settings()->arePluginsEnabled())
> +	  
extractContentTypeFromPluginList(PluginDatabase::installedPlugins()->plugins(),
&mimeTypes);

Frame::settings() is just a shortcut for Page::settings.

Lets use the later, please.

if (d->page->settings()->arePluginsEnabled()) should work. or maybe with an
additional null check for settings.

> +
> +    WebCore::Frame *frame =
d->page->focusController()->focusedOrMainFrame();
> +    if (frame && frame->settings() && frame->settings()->arePluginsEnabled()
&&
> +	   PluginDatabase::installedPlugins()->isMIMETypeRegistered(type))

Ditto.

Other than that, it looks fine to me.