User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120406 Firefox/14.0a1 Build ID: 20120406031222 Steps to reproduce: Go to imdb and try to play a trailer, either by clicking by clicking the "watch trailer" button or by clicking on a thumbnail. Actual results: The "missing add-on" notification appears. Expected results: The "click to play" notification should appear. Clicking "activate all plugins on the page" does work and activates the plugin correctly. Example : http://www.imdb.com/title/tt0425210/

Confirmed, Setting to NEW for now. Not sure if this is a dupe of bug 743102. May be that the title of 743102 should be changed and just lump all the 'need to install plugin' fails under one bug, if there is a subtle difference between each and every plugin that would need separate bugs. I'll leave it to the triagers to figure that one out.

I don't know if this has been reported yet but on vimeo the the "click to activate plugins" message which should replace the video doesn't appear. Clicking where the video should be does activate the plugins however, but only after the page is completely loaded. Testcase : http://vimeo.com/39945517

When the mime type is "binary/octet-stream" (equivalently, "application/octet-stream"), the code tries to get a more useful mime type based on the file extension and what plugins are installed and enabled. If a plugin is marked click-to-play, no mime type will be returned, and it will appear that there is no plugin for this object. This patch adds functionality to look up the mime type regardless of enabled/disabled/click-to-play status. This way, we get the right mime type and make the right decision regarding click-to-play/disabled/missing etc.

Comment on attachment 614169 [details] [diff] [review] proposed fix Review of attachment 614169 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsObjectLoadingContent.cpp @@ +800,5 @@ > + mContentType.EqualsASCII(BINARY_OCTET_STREAM)) { > + nsCAutoString ext; > + const char* typeFromExt; > + GetExtensionFromURI(uri, ext); > + nsCOMPtr<nsIPluginHost> pluginHostCOM(do_GetService(MOZ_PLUGIN_HOST_CONTRACTID)); MOZ_ASSERT(pluginHostCOM); ::: dom/plugins/base/nsPluginHost.cpp @@ +1577,5 @@ > return nsnull; > } > > +nsresult > +nsPluginHost::GetPluginTypeFromExtension(const char* aExtension, const char* &aMimeType) { This is very similar to FindPluginEnabledForExtension, with the only difference being that FindPluginEnabledForExtension checks that the plugin is enabled. Is it possible to remove the duplication here or make FindPluginEnabledForExtension use this function?

(In reply to Jared Wein [:jaws] from comment #4) > > This is very similar to FindPluginEnabledForExtension, with the only > difference being that FindPluginEnabledForExtension checks that the plugin > is enabled. > > Is it possible to remove the duplication here or make > FindPluginEnabledForExtension use this function? As it turns out, there's another section of the code that I think does the correct thing. The problem was it only looked for application/octet-stream. Adding binary/octet-stream should fix it. I'll put a better patch up.

Comment on attachment 614214 [details] [diff] [review] fix v2 Review of attachment 614214 [details] [diff] [review]: ----------------------------------------------------------------- Please make sure to request review from Josh Aas as he has much better background knowledge on plugins than I do :)

Comment on attachment 614214 [details] [diff] [review] fix v2 Why does this bug only happen when click-to-play is enabled? Your patch doesn't seem to have anything to do with click-to-play.

Empirically, it seems the code only runs nsObjectLoadingContent::OnStartRequest when click-to-play is enabled. I'm having a hard time figuring out exactly why, because it's on an input stream callback, and I can't find where it gets set up.

(In reply to David Keeler from comment #9) > Empirically, it seems the code only runs > nsObjectLoadingContent::OnStartRequest when click-to-play is enabled. I'm > having a hard time figuring out exactly why, because it's on an input stream > callback, and I can't find where it gets set up. Ok - I think I figured it out. If the plugin is enabled, AsyncStartPluginInstance() is called. Otherwise (if the plugin is click-to-play or there are other errors), a new nsIChannel is created that loads the url and calls back to OnStartRequest (around nsObjectLoadingContent.cpp:1570-1620).

Here's the try run, btw: https://tbpl.mozilla.org/?tree=Try&rev=65e9042cc3c9

Carrying over r+ for rebase.

https://hg.mozilla.org/integration/mozilla-inbound/rev/81736e2ab216 Should there be a test for this?

(In reply to Ryan VanderMeulen from comment #13) > Should there be a test for this? Probably. Do we have a way of testing using plugins other than the built-in test one?

Not really, but you could add a slimmed-down copy of the existing test plugin for use in click-to-play testing, and have it marked as click-to-play in the Mochitest profile. Alternately you could do some funky things like: * Build two copies of the test plugin with different names, but with different preprocessor defines so they handle different mime types. * Make the test plugin inspect its own filename to determine what mime types to handle, and make a copy of it with a different name after building it

How hard is it to mark a plugin click-to-play within the mochitest framework itself? Can we just temporarily mark the testplugin as click-to-play for one test and then reset it?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #16) > How hard is it to mark a plugin click-to-play within the mochitest framework > itself? Can we just temporarily mark the testplugin as click-to-play for one > test and then reset it? We actually do that now. The problem with this bug is it involves loading content with a mime type of "binary/octet-stream", and if there isn't a plugin present that can handle it (e.g. flash), it will get marked as unsupported/missing. For this and other bugs, it would be nice to have (a) more versatile test plugin(s), so I'll file a bug to make that happen (unless there is one already).

I filed bug 749257 to improve the test plugin.