| Summary: | WebKit shouldn't load a plug-in based on file extension if a MIME type is specified. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andy Estes <aestes> | ||||||
| Component: | New Bugs | Assignee: | Andy Estes <aestes> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, eric, webkit.review.bot | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | OS X 10.5 | ||||||||
| Attachments: |
|
||||||||
|
Description
Andy Estes
2010-10-21 00:34:02 PDT
In the following example: <embed src="movie.m4v" type="application/x-shockwave-flash"> when Flash isn't installed but QuickTime is, WebKit will unexpectedly load the QuickTime plug-in. This is because WebKit recognizes that QuickTime supports files ending in '.m4v' and loads the file in QuickTime despite the markup specifying a MIME type QuickTime doesn't support. In this case, WebKit should display the missing plug-in indicator if an explicit MIME type is specified that no installed plug-in claims to support. Created attachment 71401 [details]
Patch
Giving myself an r-. I need to see what other platforms need this fix in their FrameLoaderClient implementation before posting this for review. (In reply to comment #4) > Giving myself an r-. I need to see what other platforms need this fix in their FrameLoaderClient implementation before posting this for review. Looks okay. Comment on attachment 71401 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71401&action=review > WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1632 > + if (!pluginPackage && [extension length] != 0 && [MIMEType length] == 0) { Should we be using [extension length] && ![MIMEType length] and get rid of the comparisons with 0? Comment on attachment 71401 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71401&action=review r=me with the suggested changes >> WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1632 >> + if (!pluginPackage && [extension length] != 0 && [MIMEType length] == 0) { > > Should we be using [extension length] && ![MIMEType length] and get rid of the comparisons with 0? Indeed, please follow current WebKit style even though the old code is incorrect. > LayoutTests/plugins/invalid-mime-with-valid-extension-shows-missing-plugin.html:19 > I would guess that the data: url will be delivered to the plug-in much faster than the test file, so I think you have a potential race condition here even with the setTimeout(0). I think you may want to use the test file for both plug-in instances and use a non-zero timeout instead. Created attachment 71514 [details]
Patch
(In reply to comment #8) > Created an attachment (id=71514) [details] > Patch I rewrote the test. It changed enough that it probably warrants a new review. Comment on attachment 71514 [details]
Patch
Much simpler test, nice!
One very minor nit that you might want to change: you don't need to call finishTest from logFailure, just get rid of the "else" after calling testCallback.
r=me
Thanks Eric. Landed in http://trac.webkit.org/changeset/70332. Oops, forgot to svn add the tests. Layout tests landed in http://trac.webkit.org/changeset/70334. http://trac.webkit.org/changeset/70332 might have broken GTK Linux 32-bit Release http://trac.webkit.org/changeset/70334 might have broken GTK Linux 32-bit Release (In reply to comment #14) > http://trac.webkit.org/changeset/70334 might have broken GTK Linux 32-bit Release Fixing. I didn't handle this for WebKit2 or Win/Gtk/Qt (oops!). Fixed in http://trac.webkit.org/changeset/70379. http://trac.webkit.org/changeset/70379 might have broken GTK Linux 32-bit Debug |