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

[bugzilla-daemon at webkit.org]

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

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

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
Generally, looks good. Please fix kling's suggest and also add a changelog
entry.

I am asking myself if these method should go to QWebFrame. Personally I thing
they do not belong to a particularly frame of your page, so I'd rather add them
to QWebPage.

> +/*!
> + *  \since 4.8
> + * 
> + *  Returns true if the frame can handle the given mimeType, false
otherwise.
> + */
> +bool QWebFrame::isSupportedContentType(const QString &mimeType) const

Maybe naming it like: QWeb{Frame or Page}::supportsContentType(XXX) ? not sure
...

Please block the meta bug for API change in Qt4.8/QtWebKit2.1 : bug 31552

> +/*!
> + *  \since 4.8
> + *
> + *  Returns the list of all content types supported by this frame.
> + */
> +QStringList QWebFrame::supportedContentTypes() const
> +{
> +    QStringList mimeTypes;
> +    
> +   
extractContentTypeFromHash(MIMETypeRegistry::getSupportedImageMIMETypes(),
mimeTypes);
> +   
extractContentTypeFromHash(MIMETypeRegistry::getSupportedNonImageMIMETypes(),
mimeTypes);

why not making a static helper function for the block below, similarly to what
you did above?

> +    if (d->frame && d->frame->settings() &&
d->frame->settings()->arePluginsEnabled()) {
> +	   const Vector<PluginPackage*> &plugins =
PluginDatabase::installedPlugins()->plugins();
> +	   for (unsigned int i = 0; i < plugins.size(); ++i) {		  
> +	       MIMEToDescriptionsMap::const_iterator map_it =
plugins[i]->mimeToDescriptions().begin();
> +	       MIMEToDescriptionsMap::const_iterator map_end =
plugins[i]->mimeToDescriptions().end();
> +	       for (; map_it != map_end; ++map_it)
> +		 mimeTypes << map_it->first;		
> +	   }	    
> +    }
> +    
> +    return mimeTypes;
> +}