144040 - In about:plugins, the Enabled column is set incorrectly

Status: VERIFIED FIXED

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

Description

[Francois Gouget]

From Bugzilla Helper:
User-Agent: Mozilla/4.77 [en] (X11; U; Linux 2.4.17 i686)
BuildID:    2002051009

The JavaScript code used in the about:plugins page does the following:

var plugin = navigator.plugins[i];
...
enabled = "No";
enabledPlugin = mimetype.enabledPlugin;
if (enabledPlugin && (enabledPlugin.name == plugin.name))
   enabled = "Yes";

So if two plugins have the same name, they will both be marked as enabled which
is obviously incorrect. A better check for enabled would be:

if (enabledPlugin && (enabledPlugin.filename == plugin.filename))
   enabled = "Yes";

The filename is necessarily unique, but the name is not.


Reproducible: Always
Steps to Reproduce:
1. cd /usr/lib/mozilla/plugins
2. cp libflashplayer.so npflash.so
3. mozilla about:plugins


Actual Results:  Shockwave Flash

File name: /usr/local/opt/mozilla-1.0rc2/plugins/npflash.so
Shockwave Flash 4.0 r12

Mime Type Description Suffixes Enabled
application/x-shockwave-flash Shockwave Flash swf Yes
application/futuresplash FutureSplash Player spl Yes


Shockwave Flash

File name: /usr/local/opt/mozilla-1.0rc2/plugins/libflashplayer.so
Shockwave Flash 4.0 r12

Mime Type Description Suffixes Enabled
application/x-shockwave-flash Shockwave Flash swf Yes
application/futuresplash FutureSplash Player spl Yes


Expected Results:  Shockwave Flash

File name: /usr/local/opt/mozilla-1.0rc2/plugins/npflash.so
Shockwave Flash 4.0 r12

Mime Type Description Suffixes Enabled
application/x-shockwave-flash Shockwave Flash swf Yes
application/futuresplash FutureSplash Player spl Yes


Shockwave Flash

File name: /usr/local/opt/mozilla-1.0rc2/plugins/libflashplayer.so
Shockwave Flash 4.0 r12

Mime Type Description Suffixes Enabled
application/x-shockwave-flash Shockwave Flash swf Yes
application/futuresplash FutureSplash Player spl No


I know that having multiple plugins with the same name is uncommon and can cause
other problems, typically with JavaScript code that check whether a given plugin
is present or not. It is as I was analyzing such JavaScript code that I realized
that there would be a problem if simply testing the name instead of the
filename.
Even if this situation is uncommon I think the about:plugins page should be
fixed bcause:
 * the fix is quite simple
 * accuracy is paramount in the about:plugins page
 * the about:plugins page it a prime target for those who 'learn by example'

Comment 1

[Francois Gouget]

I must have been in the wrong browser.
If I do the manipulation I described, i.e. 'cp libflashplayer.so npflash.so',
then Mozilla 1.0rc2 only shows one plugin called 'Shockwave Flash'.
So I redid the test in a variety of browsers, and apparently Mozilla >=0.9.7
will only show one browser by a given name.
However Mozilla 0.9.4, Netscape 6.2.2 and Netscape 4.77 all show multiple
instances of the 'Shockwave Flash' plugin and thus exhibit the problem which
shows that the JavaScript logic is flawed.

This is why I still think it is more correct to check the filename. I did verify
that this fixes the problem in Netscape 4 (for Mozilla, bug 144044 gets in the way).

Comment 2

[Francois Gouget]

> apparently Mozilla >=0.9.7 will only show one browser by a given name.
I meant 'one plugin by a given name'.

Btw, is that a bug or a feature? It seems other browsers seem to have no problem
reporting more than one plugin by a given name (e.g. Netscape 4 & Opera), i.e.
they will list both 'libflashplayer.so' and 'npflash.so' even though they have
the same name.

Comment 3

[rubydoo123]

this must be fixed for the plugin manager so that Javascript can tell what
plugin the browser is going to choose. 

Comment 4

[Alexey Chernyak]

Attachment: patch

fix + clean up

Comment 5

[Alexey Chernyak]

taking the bug

Comment 6

[Johnny Stenback (:jst)]

I want peterl to review this before I sr, once he approves I'll sr...

Comment 7

[Peter Lubczynski]

r=peterl

...bug I think it may be just better to remove this field because it's
misleading. See bug 144044.

Comment 8

[Johnny Stenback (:jst)]

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

sr=jst

Comment 9

[Asa Dotzler [:asa]]

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

a=asa (on behalf of drivers) for checkin to the 1.1 trunk.

Comment 10

[Johnny Stenback (:jst)]

Fix checked in.

Comment 11

[Jeremy M. Dolan]

> The filename is necessarily unique, but the name is not.

That isn't true at all:

% ls -l /opt/mozilla/7-08/plugins /home/mozilla/.mozilla/plugins
/home/mozilla/.mozilla/plugins:
total 936
-r--r--r--    1 mozilla  mozilla      2363 Jun 20 11:08 ShockwaveFlash.class
-rwxr-xr-x    1 mozilla  mozilla    948056 Mar 25 12:17 libflashplayer.so
lrwxrwxrwx    1 mozilla  mozilla        51 Mar 11 21:18 libjavaplugin_oji140.so
-> /usr/java/plugin/i386/ns610/libjavaplugin_oji140.so

/opt/mozilla/7-08/plugins:
total 20
-rwxr-xr-x    1 jmd      jmd         19396 Jul  8 08:40 libnullplugin.so

There are two plugin locations. Perhaps filename and plugin name should both be
checked? Perhaps we should go back to just checking names, and in the case of
two plugins with the same name, only one is loaded? Seems like a bug that it's
possible to load two copies of flash in the first place.

And, if this is implemented, remember the plugin in the users profile directory
should override the system directory always.

Comment 12

[Francois Gouget]

I have not yet tested 1.0.1 but the above patch looks good to me.


> > The filename is necessarily unique, but the name is not.
>
> That isn't true at all:
[...]

True, you can have two plugins with the same filename in two different
directories. However I meant "plugin.filename" which is necessarily unique since
it contains the full path.
Of course, there is still the possibility that a single plugin could be known
under two different names due to either symbolic or hard links or even basic
copying. But that's hardly a problem (or even relevant from the browser's point
of vue, the full path is what it ultimately dlopens).


> There are two plugin locations. Perhaps filename and plugin name should
> both be checked? Perhaps we should go back to just checking names, and
> in the case of two plugins with the same name, only one is loaded?
> Seems like a bug that it's possible to load two copies of flash in
> the first place.

It's not a problem of loading two plugins simultaneously. The problem is when
you try to specify which plugin is going to be used for a given MIME type. This
is the same problem that happens in Netscape 4.x when the user specifies which
plugin to lod for a given MIME type (this capability is not there yet in Mozilla
but will be as soon as bug 19118 is resolved).

To ilustrate the problem, here is what happens in Netscape 4.x: you start
Edit/Preferences/Navigator/Applications, select a MIME type such as
application/x-shockwave-flash and specify that you want it to be handled by the
plugin called 'Shockwave Flash'. Netscape sutifully stores this information in
~/.mailcap, it looks something like:

application/x-shockwave-flash;;\
        x-mozilla-flags=plugin:Shockwave Flash

Now, you get to a page that needs the Flash plugin, which plugin is to be
loaded: /usr/lib/netscape/plugins/libflashplayer.so which is called 'Shockwave
Flash' or '~/.netscape/plugins/npwine.NP32SWF.so' which is called 'Shockwave
Flash'? The way we 'resolved' this is that we renamed our plugin to 'shockwave
flash (CrossOver - NPSWF32.dll)' to remove the confusion (and then many pages
fail to realize that a Flash plugin is installed because they use broken
JavaScript code that relies on the plugin name, but that's a separate issue).


> And, if this is implemented, remember the plugin in the users profile
> directory should override the system directory always.

No. The user should be able to specify which plugin he wants to use on a per
MIME-type basis. But again, this is the subject of bug 19118.

Comment 13

[Alexey Chernyak]

verified fixed