Currently we have a preference mailnews.message_display.allow.plugins that globally disables or enables plugins within mailnews displays, e.g. 3-pane window, compose window etc. Various users want to be able to use plugins within RSS feeds, but probably not necessarily within emails, and likewise, with the use of content tabs, there may be a case for extension wanting to use plugins. I have a WIP patch that I think will allow us to enable plugins in RSS feeds and content tabs, but have the pref above only disable plugins in messages. If users then want to completely disable plugins, they they can still do so via the add-on manager or the plugin.disable preference. One of the side-effects of this is that we should be able to get reftests passing on Thunderbird (they assume plugins are enabled), I've not worked out how useful that is yet, but it may be useful information around release times.
Created attachment 513303 [details] [diff] [review] WIP This patch implements the changes required. I've got a couple more tweaks to do on the mozmill tests, but it should be pretty much there now. I'll detail more about it when I request review, more posting just to keep it safe.
Ok, Mozmill tests currently fail in packaged format because they don't have the test plugin installed by default. I'll work on resolving how to fix that.
Comment on attachment 513468 [details] [diff] [review] The fix This looks fine, esp if you use an actual out param for the docshell thing..
Comment on attachment 513468 [details] [diff] [review] The fix 2011? + * Portions created by the Initial Developer are Copyright (C) 2001 + * the Initial Developer. All Rights Reserved. This certainly looks more sane that what we had before...
Created attachment 515034 [details] [diff] [review] The fix v2 Changed the docShell parameter to be a proper out param.
Comment on attachment 515034 [details] [diff] [review] The fix v2 The nsMessenger changes I understand. In fact, I wouldn't be surprised if they turned out to be unnecessary all along, since as far as I can tell the content policy blocks all plugins in mail windows anyway. The nsMsgContentPolicy changes I don't understand. I could understand it if either a) plugins were disabled at the same time as script, and then potentially enabled at the same time as script, or b) content policy was tweaked to only apply plugin policy to mail/news URLs. But this patch seems to have a hybrid approach, with the plugins being disabled during the plugin content policy check (which sounds like a really bad idea to me) and no sign of them being enabled. EDIT: so I ran this through a debugger, and it showed me that GetDocshellForDisabling was throwing in the plugin case, which turns out to be unsurprising, since the only objects that have frame loaders are frames (well, any embedded objects with HTML/XML content, but that still excludes plugins). So what's really happening is this: 1. If the plugin pref is enabled, then the plugin is allowed. 2. Otherwise if the load isn't owned by a mail/news URL, then GetDocshellForDisabling returns a null docShell, and the plugin is allowed. 3. Otherwise GetDocshellForDisabling fails, and the plugin is only blocked because of the "fallback" code.
Created attachment 515607 [details] [diff] [review] The fix v3 Neil's right, and setting it on document load seems much more logical as well. I've also updated onLocationChange so that it sets plugins on/off as necessary, this fixes the case of email message to non-email message being displayed in the message pane. I've also added an extra test for that specific case.
Comment on attachment 515607 [details] [diff] [review] The fix v3 Re-requesting review from David in case he wants to take another quick look.
Created attachment 515686 [details] [diff] [review] The fix v4 I was just testing this on try server and realised that putting code to always be executed inside assertions was not a good idea.
Comment on attachment 515686 [details] [diff] [review] The fix v4 looks ok, though the copyright still says 2001 passes mozmill tests on a release build for me.
Plugins still seem to be working in news posts _without_ setting the new pref: mailnews.message_display.allow_plugins to "true" Didn't try mail, and the old pref seems to still be there. I'm assuming this made today's nightly because of the existence of the new pref. Mozilla/5.0 (Windows NT 5.0; rv:2.0b13pre) Gecko/20110302 Thunderbird/3.3a3pre ID:20110302044201
(In reply to comment #13) > Plugins still seem to be working in news posts _without_ setting the new pref: > mailnews.message_display.allow_plugins to "true" That's strange. Can you file a follow-up bug on this and point me at a suitable test newsgroup? > Didn't try mail, and the old pref seems to still be there. The old pref will only be there if you've previously set it to the non-default value.
My mistake..The pref was being ignored because of an experimental extension that I'm testing. Anyhow, I posted a test at news://news.mozilla.org:email@example.com