[Qt] Don't rely on QMimeDatabase for well-known extension to MIME type mapping.
Created attachment 177202 [details] Patch
Comment on attachment 177202 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177202&action=review > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:87 > String MIMETypeRegistry::getMIMETypeForPath(const String& path) > { > + size_t pos = path.reverseFind('.'); > + if (pos != notFound) { > + String extension = path.substring(pos + 1); > + String result = getWellKnownMIMETypeForExtension(extension); > + if (result.length()) > + return result; > + } > + Why is the change in getMIMETypeForPath? it is getMIMETypeForExtension that is used to determine if we should/can load content.
Btw, I think we should add all the aliases of the supported mime-types so that we don't end up in that situation.
(In reply to comment #2) > (From update of attachment 177202 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177202&action=review > > > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:87 > > String MIMETypeRegistry::getMIMETypeForPath(const String& path) > > { > > + size_t pos = path.reverseFind('.'); > > + if (pos != notFound) { > > + String extension = path.substring(pos + 1); > > + String result = getWellKnownMIMETypeForExtension(extension); > > + if (result.length()) > > + return result; > > + } > > + > > Why is the change in getMIMETypeForPath? it is getMIMETypeForExtension that is used to determine if we should/can load content. It's used in QNetworkReplyHandler::sendResponseIfNeeded when loading local files.
It seems some tools used for creating associating mimetypes to application will dynamical invent new mimetypes such as application/x-extension-html, which can confuse the mimedatabase because we only rely on the first match. We need to ensure users can not shoot themselves this hard with the basic mimetypes. For images and other types it can be potentially useful, but for html and other essential types it can only break stuff.
Created attachment 177443 [details] Patch A condensed version of Pierre's patch only detecting the essential mimetypes and doing it for both getMimetype functions
Comment on attachment 177443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177443&action=review > Source/WebCore/ChangeLog:9 > + Extend the short static list to also include MIME types essential WebKit, to WebKit > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:59 > + { "xls", ".xls", "text/xsl" }, I didn't know we supported Excel sheets with WebKit :P > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:76 > + // FIXME: We should get all the matched mimetypes with mimeTypesForFileName, and prefer one we support. mimeTypesForFileName -> mimeTypesForFile? > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:77 > + // But initializeSupportedImageMIMETypes will first have to stop using getMIMETypeForExtension. Could you write why? I don't have enough context to understand this part.
(In reply to comment #7) > (From update of attachment 177443 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177443&action=review > > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:76 > > + // FIXME: We should get all the matched mimetypes with mimeTypesForFileName, and prefer one we support. > > mimeTypesForFileName -> mimeTypesForFile? > Well, that is actually the name of this particular method in QMimeDatabase ;) > > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:77 > > + // But initializeSupportedImageMIMETypes will first have to stop using getMIMETypeForExtension. > > Could you write why? I don't have enough context to understand this part. Because to tell if a mime-type is supported we have to check against the supported mime-types, but we can not do that while we are still in the process of initializing the supported mime-types.
Created attachment 177707 [details] Patch
Comment on attachment 177707 [details] Patch (In reply to comment #8) > Well, that is actually the name of this particular method in QMimeDatabase ;) Ah! I see what you mean, sorry about that.
Comment on attachment 177707 [details] Patch Clearing flags on attachment: 177707 Committed r136669: <http://trac.webkit.org/changeset/136669>
All reviewed patches have been landed. Closing bug.