90062 - stop excluding the 4.x plugin directory

Status: RESOLVED INVALID

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

Description

[Chris Waterson]

cf. bug 89407. peterl explained to me that this stuff was put in so that we
_wouldn't_ load plugins from the 4.x folder (except for the most popular plugins
in the school). That just seems bass-ackwards to me. We should load _any_ plugin
that we find in the 4.x directory (_maybe_ with the exception of Java).

Comment 1

[Peter Lubczynski]

Attachment: patch to let all 4.x plugins through except for Java

Comment 2

[Peter Lubczynski]

Attachment: real patch (correctly excluding java)

Comment 3

[Peter Lubczynski]

That last patch is for the trunk to let the 4.x plugin flood gates through!

Andrei, I'm getting some weirdness with over-lap. This could be that when we
compare the filenames to determine if they are dups, we compare the whole path.
Just the filename should be compared. I'll take a closer look and open a
seperate bug on this if it's a problem as it could possibly be branch material
in case plugin vendors do update their installers.

Comment 4

[Peter Lubczynski]

never mind, the the patch looks good on Win32. I will try Mac.

Comment 5

[av (gone)]

There is also a static function IsJavaPlugin which does the same job but using 
filename instead of mimetype. We should probably remove it as it is not used.

r=av

I didn't understand what you said abour overlap. I think we compare leaf names 
only in areTheSameFileNames().

Comment 6

[av (gone)]

Attachment: second try

Comment 7

[av (gone)]

I also changed '>=' to '==' Why '>='?

Another thought, if we are so confident that Java is the only plugin we do not 
want, maybe to reflect this fact in the function name?

Comment 8

[av (gone)]

Ah... I see why '>=' ! Sorry for the spam.

Comment 9

[av (gone)]

Attachment: third try -- back to peterl's java mime type handling

Comment 10

[Peter Lubczynski]

>> r=av

Which patch (for the super reviewer)? Do you want me to check this in or do you
want to?

>> I didn't understand what you said abour overlap. I think we compare leaf 
>> names only in areTheSameFileNames().

Okay, I think I got this mixed up with a problem on the Mac. Comparing the leaf
names works on Windows because plugins usually keep the same 8.3 DLL name, even
accross folders. On Mac, however, filenames aren't a good way to compare if
files are the same which is probably why I see dups on Mac from the System
Internet Plugins Folder. I'll do some more investigation and open a bug if there
is a problem. I think we have a better way of checking if it's a dup on Mac.

Comment 11

[av (gone)]

http://bugzilla.mozilla.org/showattachment.cgi?attach_id=41985 is what to my 
mind should go in. It is essentialy yours, I just removed ghost function.

Chris, can you sr this?

Comment 12

[av (gone)]

Oh man... We should never do things in hurry.

if(PL_strcasecmp(tag->mMimeTypeArray[i], "application/x-java") >= 0 )

is still wrong. It should be

if(PL_strncasecmp(tag->mMimeTypeArray[i], "application/x-java", 18) == 0 )

Comment 13

[av (gone)]

Attachment: fourth try

Comment 14

[Peter Lubczynski]

Okay, I like that last one. r=peterl But why is the "ghost" function not needed
anymore?

Comment 15

[av (gone)]

Originally we just did it for java (exclude from the 'sweep'), hence this 
function. When we decided to do the sweep only for specific plugins more 
universal isUnwantedPlugin() came up. IsJavaPlugin() became abandoned at that 
point and should have been removed then.

Comment 16

[Mike Shaver (:shaver emeritus)]

No, the latest patch will cause us to skip any MIME type that starts with
application/x-java, which isn't really appropriate: there's no reason that
someone's application/x-javascript-debugger plugin shouldn't be loaded.

Why not just enumerate the MIME types we can't load, rather than relying on a
common prefix?  (Or figure out how to load the 4.x Java plugin, maybe -- does it
not follow the 4.x plugin API that we're trying to emulate?)

Comment 17

[sean echevarria]

One of the reasons for not picking up 4x plugins is that you end up with 
situations like bug 59935 for every scriptable 4x plugin.

Can you modify about:plugins to document for the end user whether or not a 
listed plugin is scriptable and therefore REALLY supported?

Comment 18

[Peter Lubczynski]

The check for Java will have to be done in a smarter way like in isJava().

I like Sean's idea. I think about:plugins is a chrome:// delivered page so we
can probably find some way of checking if the plugin is scriptable in Javascript
through using the plugin host.

Comment 19

[Peter Lubczynski]

So there is now a preference to control this and to see the feasibility of
sweeping all 4.x plugins:

Enable ALL plugins from the 4.x folder
pref("plugin.scan.4xPluginFolder", true);

Don't allow ANY plugins from the 4.x folder
pref("plugin.scan.4xPluginFolder", false);

Without any pref, the default remains: sweep just the tier1 plugins from the 4.x
folder.

So far, I've heard that if we simply allow all plugins, we may have problems
with the following:
Beatnik (using unfrozen XPCOM interfaces and may crash)
Java (differenet kind of API for Mozilla)
Real (differenet kind of API for Mozilla needed for scripting support)
Default Plugin (we've got our own)

If we want to fix this bug, we may need to turn our whitelist of tier1 4.x
plugins to a black list of plugins we don't want to include.

Comment 20

[rubydoo123]

this is dependent upon the plug-in manager work, marking as such

Comment 21

[Josh Aas]

This bug no longer makes any sense.