With the recent addition of QMimeDatabase in QtCore we can get rid of the static table of mimetypes and extensions which is constantly in risk of becoming outdated and instead use QMimeDatabase which uses the system mimetype database if available. Note that to begin with this should probably be restricted to filename detection only. Detection by content has historical had some security issues due to some loaders not verifying against context. Benifits should be better detection of downloaded files (offers of applications), and ability to recognize common aliases for supported mimetypes.
Created attachment 174926 [details] Patch
Comment on attachment 174926 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174926&action=review There's a lot of cross-platform code changed. Is there a way to avoid that and make our MIMETypeRegistry.cpp behave more like the other platforms? > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:42 > +inline static QMimeDatabase& mimeDatabase() > +{ > + DEFINE_STATIC_LOCAL(QMimeDatabase, s_mimeDatabase, ()); > + return s_mimeDatabase; > +} I don't think you need this optimization. AFAICS QMimeDatabase is really cheap to construct because it internally uses exactly the same pattern already.
(In reply to comment #2) > (From update of attachment 174926 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=174926&action=review > > There's a lot of cross-platform code changed. Is there a way to avoid that and make our MIMETypeRegistry.cpp behave more like the other platforms? > We still behave the same for most parts. The only behaviour change is the attempt to detect multi-part suffixes (.tar.gz), and this code was changed so that other platforms can do the same if they want, but it is kind of a Unix tradition only. > > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:42 > > +inline static QMimeDatabase& mimeDatabase() > > +{ > > + DEFINE_STATIC_LOCAL(QMimeDatabase, s_mimeDatabase, ()); > > + return s_mimeDatabase; > > +} > > I don't think you need this optimization. AFAICS QMimeDatabase is really cheap to construct because it internally uses exactly the same pattern already. Why aren't the QMimeDatabase method static then?
Created attachment 174959 [details] Patch Removed static QMimeDatabase optimization, ensured WK2 uses the same logic as WK1 and added one mime-type feature from Qt backend
Comment on attachment 174959 [details] Patch Attachment 174959 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14910178
Created attachment 174960 [details] Patch Update exported symbols
Comment on attachment 174960 [details] Patch Attachment 174960 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14905192
Created attachment 174961 [details] Patch Update exported symbols
Created attachment 175637 [details] Patch Use cross-platform mimetype detection in File.cpp. Added another missing function in the Qt implementation to detect if extensions are valid for a given mimetype, and used that to set suggestedFilename
Created attachment 175638 [details] Patch Forgot to commit the last two lines
Created attachment 175645 [details] Patch Split patch in three. This is the first part only detecting by last extension but doing so using QMimeDatabase
Comment on attachment 175645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175645&action=review r=me with comments. Nice to see the list of hardcoded types finally go away! > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:41 > + const String filename = "filename." + ext.lower(); Since the only thing we do is to take the filename and convert it (allocatingly) to a QString, I suppose you could also construct a QString here right away. > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:56 > + return "application/octet-stream"; Why not return defaultMIMEType() here and safe the string construction? :)
Committed r135507: <http://trac.webkit.org/changeset/135507>
new bug report to track the regression caused by this patch - https://bugs.webkit.org/show_bug.cgi?id=103069