191021 - Put the null plugin back where it belongs

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Stephen Walker]

With today's Windows installer build, the null plugin isn't being displayed when
a plugin can't be found; about:mozilla doesn't display anything about the null
plugin.  The null plugin is located in the GRE plugins dir, but it appears
Mozilla isn't scanning that dir for plugins.

Comment 1

[Doug Turner (:dougt)]

do we really want the default plugin in the GRE ?

Comment 2

[Peter Lubczynski]

hmm...different embedders may have overriden their default plugins with their
own implementations so I think we may need to scan the GRE dir in addition to
the app plugins dir but keep the null plugin in the app dir. Also, another
embedding project has asked for a sharable location for plugins so maybe we can
have a system global folder like on Mac or GRE global that embedders can opt in?

Here's the code in nsAppFileLocationProvider that causes the scan of bin/plugins:

197     else if (nsCRT::strcmp(prop, NS_APP_PLUGINS_DIR) == 0)
198     {
199         rv = CloneMozBinDirectory(getter_AddRefs(localFile));
200         if (NS_SUCCEEDED(rv))
201             rv = localFile->AppendRelativeNativePath(PLUGINS_DIR_NAME);

http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsAppFileLocationProvider.cpp#583

Comment 3

[Doug Turner (:dougt)]

wouldn't it be nice if the plugin code just registered their own directory
service provider :-)

Comment 4

[Peter Lubczynski]

it could (and I recall it did at one point in time) but embedders requested
finer control

Comment 5

[timeless]

Attachment: put the null plugin back where it belongs

the null plugin we're packaging is really really horrible, we should be
encouraging embedders to replace it by making them opt into using it.

Comment 6

[timeless]

we don't want the null plugin in the gre.

Comment 7

[Peter Lubczynski]

Comment on attachment 112926 [details] [diff] [review]
put the null plugin back where it belongs

r=peterl

Comment 8

[timeless]

Comment on attachment 112926 [details] [diff] [review]
put the null plugin back where it belongs

this is a small build change but it's important for the gre not to include this
file.

Comment 9

[Asa Dotzler [:asa]]

Comment on attachment 112926 [details] [diff] [review]
put the null plugin back where it belongs

a=asa (on behalf of drivers) for checkin to 1.3beta.

Comment 10

[timeless]

checked in

Comment 11

[timeless]

Attachment: really put it back where it belongs

*sigh* why oh why are the gre packaging files pathwise incompatible with the
mozilla packaging files?

Comment 12

[Sean Su]

Attachment: patch to remove code that deletes npnul32.dll

timeless, you also need to check this patch in.  browser.xpi should no longer
be deleting the npnul32.dll during installation because it's installing it now.

Comment 13

[timeless]

right, thanks.
drivers: the gre changes currently mean we aren't shipping a null plugin on
windows. given that the null plugin can crash in edge cases that might not be a
bad thing, but most people would probably be annoyed by the you're missing the
null plugin dialog. and it's a usability loss.  the patch pair here is simple
and imo worth committing.

Comment 14

[timeless]

Comment on attachment 113933 [details] [diff] [review]
really put it back where it belongs

this correctly packages the null plugin.

Comment 15

[timeless]

Comment on attachment 113938 [details] [diff] [review]
patch to remove code that deletes npnul32.dll

this prevents the installer from killing the null plugin ... ... ...

Comment 16

[Asa Dotzler [:asa]]

Comment on attachment 113933 [details] [diff] [review]
really put it back where it belongs

a=asa (on behalf of drivers) for checkin to 1.3b.

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 113938 [details] [diff] [review]
patch to remove code that deletes npnul32.dll

a=asa (on behalf of drivers) for checkin to 1.3beta.

Comment 18

[timeless]

checked in

Comment 19

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

clearing blocking1.3b?, as the patch is checked in

Comment 20

[bmartin]

Marking Verified Fixed.

Compared the two patches to the checkins:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=packages-win&root=/cvsroot&subdir=mozilla/xpinstall/packager&command=DIFF_FRAMESET&rev1=1.250&rev2=1.251

and

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=packages-win&root=/cvsroot&subdir=mozilla/xpinstall/packager&command=DIFF_FRAMESET&rev1=1.249&rev2=1.250