282106 - Need ability to reliably disable full page plugins

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Ben Goodger (use ben at mozilla dot org for email)]

Using the pref plugin.disable_full_page_plugin_for_types, disable full page
plugin for the specified types. This needs to be implemented in the plugin host
impl for reliability.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 174198 [details] [diff] [review]
patch

+      if (overrideTypes.IsEmpty() || !FindInReadable(mimeType, start, end)) {

No need for the IsEmpty() check there, if the string is empty FindInReadable()
won't find what you're looking for anyways.

r+sr=jst

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

won't this disable text/x-foobar even if the list only contains text/x-foo?

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

ok, how about this (assuming a more rigidly delimited list)

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

maybe it would be more user-friendly to pre- and append the , in the code
instead of requiring it to be present in the pref? then again, probably few
people will set it manually...

also, Append(',')/Assign(',') is probably a bit cheaper than AssignLiteral

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

still, wouldn't hurt to have it in the same csv format as everything else, so
here's a revised version with the 1,2,3 format reinstated.

Comment 7

[Boris Zbarsky [:bzbarsky]]

That patch doesn't seem to handle changes in the preference value...

Also, given that this code relies on this preference, it would make sense to
move the code that changes the preference (i.e. disables or enables a plugin for
a given type) into the plugin manager as well.  That would also make it easier
to implement disabling some plugin altogether...

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

This function seems to be called every time content of a type that cannot be
handled internally is loaded, so no dynamic-ness was required. 

Comment 9

[Boris Zbarsky [:bzbarsky]]

It is?  What's the callstack?  That seems very wrong.

Comment 10

[Ben Goodger (use ben at mozilla dot org for email)]

Oh, sorry, my bad. It happens once per session only. 

When my UI unsets the default handling, I remove the plugin category handler
using the category manager. 

An API for plugin enabling would be useful, but I think that's slightly separate
from this bug. 

Comment 11

[Boris Zbarsky [:bzbarsky]]

Please document the dependency on the UI in this code (and file a followup bug
to remove said dependancy).

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 175084 [details] [diff] [review]
patch

r+sr=jst

Comment 13

[Boris Zbarsky [:bzbarsky]]

Just to elaborate on comment 11, Firefox is not the only user of the plugin
manager, so we shouldn't have proper behavior of the plugin manager depending on
the Firefox UI...

Comment 14

[Mike Connor [:mconnor]]

Let's get this sorted out/landed.

Comment 15

[Mike Schroepfer]

Approaching b1 - not clear why we need this.  JST nom that patch if appropriate for 1.8.1

Comment 16

[Bastien Nocera]

(In reply to comment #15)
> Approaching b1 - not clear why we need this.

Bug 306867 is a much better one to fix, IMO, as it allows the policy to be on the plugin side. The plugin should obviously know better which files it should handle in "full screen" mode.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Bastien, this bug is about the _user_ choosing to handle a particular kind of data with a helper app instead of a plug-in.  It has nothing to do with bug 306867, which is a technical issue.

Comment 18

[Benjamin Smedberg]

I really don't think this is worth fixing. Users have the ability to disable the plugin altogether (using the addon manager), but I don't think the complexity of trying to configure fullpage separately from that is worth the complexity of code.