Need ability to reliably disable full page plugins

RESOLVED WONTFIX

Status

()

Core
Plug-ins
P1
normal
RESOLVED WONTFIX
13 years ago
4 years ago

People

(Reporter: Ben Goodger (use ben at mozilla dot org for email), Assigned: Ben Goodger (use ben at mozilla dot org for email))

Tracking

Trunk
mozilla1.8beta1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Created attachment 174198 [details] [diff] [review]
patch
Status: NEW → ASSIGNED
Flags: blocking-aviary1.1+
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta1
Attachment #174198 - Flags: review?(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
Attachment #174198 - Flags: superreview+
Attachment #174198 - Flags: review?(jst)
Attachment #174198 - Flags: review+
won't this disable text/x-foobar even if the list only contains text/x-foo?
Created attachment 175066 [details] [diff] [review]
patch

ok, how about this (assuming a more rigidly delimited list)
Attachment #174198 - Attachment is obsolete: true
Attachment #175066 - Flags: superreview?(jst)
Attachment #175066 - Flags: review?(jst)
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
Created attachment 175084 [details] [diff] [review]
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.
Attachment #175066 - Attachment is obsolete: true
Attachment #175084 - Flags: superreview?(jst)
Attachment #175084 - Flags: review?(jst)
Attachment #175066 - Flags: superreview?(jst)
Attachment #175066 - Flags: review?(jst)
Attachment #174198 - Flags: superreview+
Attachment #174198 - Flags: review+
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...
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. 
It is?  What's the callstack?  That seems very wrong.
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. 
Please document the dependency on the UI in this code (and file a followup bug
to remove said dependancy).
Comment on attachment 175084 [details] [diff] [review]
patch

r+sr=jst
Attachment #175084 - Flags: superreview?(jst)
Attachment #175084 - Flags: superreview+
Attachment #175084 - Flags: review?(jst)
Attachment #175084 - Flags: review+
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...

Updated

12 years ago
Flags: blocking-aviary1.5+ → blocking-aviary2.0?
Let's get this sorted out/landed.
Flags: blocking-firefox2? → blocking1.8.1+

Comment 15

11 years ago
Approaching b1 - not clear why we need this.  JST nom that patch if appropriate for 1.8.1
Flags: blocking1.8.1+ → blocking1.8.1-

Comment 16

11 years ago
(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.
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.

Updated

7 years ago
Flags: blocking1.9.0.19?
Flags: blocking1.9.0.19?

Updated

7 years ago
Flags: blocking1.9.0.19?

Updated

6 years ago
Flags: blocking1.9.0.19?

Updated

6 years ago
Flags: blocking1.8.1-
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.