119056 - QuickLaunch: plugins don't get refreshed after browser is restarted

Status: VERIFIED DUPLICATE of bug 133282

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

Description

[shrirang khanzode]

dunno if this will be quicklaunch or plugins component.

I noticed this on 01008 trunk build on win 98.
I had quicklaunch running (you can enable it from Edit+Prefs+Advanced menu)
steps:
Make sure quicklaunch is runnig when u launch the browser( system tray icon 'N')
1 Do not have shockwave files installed in 4.x folder or anywhere.
2 launch 6.x and go to shockwave,click on say "Inklink" or any other game that 
  requires shockwave plugin.
3. Download the shockwave installer when instructed.
4 Launch the installer, current browser windows close and a part of the 
install completes. When a new browser iwndow is launched, I get  a default 
plugin dialog. Seems like the browser needs to close completely for the new 
plugin to register.

When I tried the same steps without quicklaunch enabled, things worked fine.

Comment 1

[Matthias Versen [:Matti]]

The Plugin installations needs a restart.
You can also call about:plugins and the plugin directory is rescanned.

Comment 2

[shrirang khanzode]

true..but not everyone will know this. need to release note when the time comes 
if we cannot get a solution for this.

Comment 3

[Peter Lubczynski]

Macromedia may need to be evangelized about our Quick Launch mode or we may need
to manually do a plugins.refresh when starting up in ql mode.

Comment 4

[Peter Lubczynski]

Actually, I think this is a bug with our Quick Lauch feature. When starting a
new browser session while using this feature, the plugins need to be reloaded
like they are at startup. Otherwise, newly installed plugins aren't picked up.

This could be simple to fix if all what's needed is a call to
javascript:navigator.plugins.refresh(1) which forces all running plugins to shut
down and the plugin paths to be re-scanned for changes.

Changing summary from: [if quicklaunch is running, shockwave installation does
not finish and default plugin is seen] and reassinging

Comment 5

[Bill Law]

Peter, can you give a C++ equivalent of that JS snippet?

How about advice on the priority/severity of this bug?

Comment 6

[Peter Lubczynski]

You can call nsIPluginManager::ReloadPlugins(1) which will do pretty much the
same thing, EXCEPT clean up the DOM navigator.plugins and navigator.mimetypes
arrays. I'm don't fully understand all the nits about Quick Lauch so maybe if it
destorys the navigator JS object when the last window is gone, it's not an issue.

This bug is pretty serious to anyone who expects plugins to be refreshed upon
restarting the browser. This could be a user or a plugin vendor's installer.

Comment 7

[Peter Trudelle]

->blaker, nsbeta1+

Comment 8

[Blake Ross]

When trying to reproduce, "current browser windows" don't close as indicated in
step 4...?

Comment 9

[Blake Ross]

I don't really understand this.  As far as I can tell, the Shockwave installer
makes no attempt to close existing browser windows (and the installation
instructions doesn't say it will).  I _do_ see this bug in the new window that
it opens, but if it doesn't attempt to close all the windows first then I can't
see how quicklaunch would play a role.  Indeed, I can reproduce this with
quicklaunch off...

Comment 10

[Peter Lubczynski]

What happens if you manually exit the browser (with QuickLauch on) and run the
installer? I think their installer launchs the browser if it's set as the
default browser for the system.

For that matter, any plugin dropped in the plugins folder won't get picked up
when the user thinks they have just started the browser.

Comment 11

[Blake Ross]

I tried manually exiting the browser with QL on and running the installer.  Upon
finishing, the installer opened a new browser window (as expected) and took me
to some page at macromedia.com. Shockwave on that page worked fine.  I don't
think there's a quicklaunch bug here...?

Comment 12

[Blake Ross]

Peter, what are your results for that test?

Comment 13

[Blake Ross]

Peter?

Comment 14

[shrirang khanzode]

i will try it asap...

Comment 15

[shrirang khanzode]

news: 
tried this on win98. 'existing browser windows' (including 4.x and 6.x) closed 
for me...then I actually got a mesage saying " we have detected that u rae 
running one or more instance...blahhhh". Clse it to continue". I closed netscp6 
from the sys tray and things went ok. why did I not see this message the first 
time..hm. close this?

Comment 16

[Blake Ross]

Thanks for retesting.  Well, it's weird, when it installs for me it doesn't make
any attempt to close open browser windows it seems.  I wonder if it's different
for windows XP?

Comment 17

[Peter Lubczynski]

Sorry, I've been working on branch stuff and need to test on a clean system with
the installer. I just got it set up but havn't downloaded the installer to try.

But I changed the summary of the bug to not about about a particular plugin's
installer, but more a general problem. Not all installers popup the browser
after installing. Some only do it if we are the default browser, others don't do
it at all.

For example in this senero where a user exit's the browser while quick lauch is
on and installs a plugin. The installer will drop the DLL in the user's plugins
folder and then maybe startup the default browser for the system. What happens
if IE is the default browser?

If it doesn't show the problem, I can create a simplified plugin with testcase.

If we are running in QuickLauch, next time the user starts browsing, the plugin
will not be enabled as it would on a real browser start. It will not be in the
DOM plugin or mime type array. The user will see the default plugin again.

Once you visit about:plugins for the first time, however, things will be corrected.

I can understand not wanting to scan for plugins with QuickLauch for performance
reasons and there are other workarounds, but then it needs to be documented to
plugin vendors that restarting the browser with our QuickLaunch feature enabled
will not refresh plugins.

Comment 18

[shrirang khanzode]

Testing on win xp gives me weird results ( exactly as mentioned in the 
first comment)

On XP: (quicklaunch ON, 0204 build set as DEFAULT browser)
installed trunk 0204
Had an instance of 4.x running in the background
launched trunk build and went to shockwave.com and clicked on 'inklink'
'Download shockwave player' message appeared on the page
I clicked on it, saved the installer file 
now, without closing any browser window, I launched the installer (let the 
default browser selection 'as-is' [4.x and 0204 trunk])
after completing partially , existing 4.x window closed first, then I actually 
saw the mssage ' u have one or more instance .." , I clicked OK...but did not 
close the browser from su=ys tray ...and to my surprise..the installation 
proceeded...( on w98, I was not allowed to go ahead unless I closed quicklaunch)

On xp, the browser started and went to the shockwave page, it again popped up 
the default plugin dialog. I am lost........

Comment 19

[Peter Lubczynski]

Attachment: test about:plugins without reloading plugins

Use this helpful testcase to see what the DOM see's and not any side-effects.
Plugin loading also uses this list.

Comment 20

[Peter Lubczynski]

Attachment: ignore last, wrong file

Comment 21

[sairuh (rarely reading bugmail)]

over to quicklaunch component.

Comment 22

[Bill Law]

Resetting target milestone; this didn't get to me in time for mozilla0.9.9.

Comment 23

[Peter Trudelle]

re-assigning 'several bugs at once' to [e-mail].

Comment 24

[Jaime Rodriguez, Jr.]

ADT2 per ADT/Nav triage.

Comment 25

[Peter Trudelle]

What progress has been made toward fixing this defect?  Do we need to reassign
it, or get someone else involved to resolve it?

Comment 26

[Peter Trudelle]

We need a new owner for this, preferably someone with plugin expertise.

Comment 27

[Peter Lubczynski]

Here, I'll take it. But one important question for QuickLaunch gurus:

Can I register for a notification with QuickLaunch on when the last window is
closed or the first one is opened? I need to know when this condition of
"running in the background" ends or begins.

Thanks.

Comment 28

[Brian Ryner (not reading)]

The "session-logout" notification (via nsIObserverService) happens when the last
window is closed.

Comment 29

[rubydoo123]

pushing this out to 1.2, this one will have to wait until more pressing issues 
get resolved

Comment 30

[Peter Trudelle]

Bryner, would you be able to fix this?  You obviously know what needs to be
done, and Beppe says that PeterL should have time to consult and review it.

Comment 31

[Brian Ryner (not reading)]

No, I don't know how to fix this, exactly.  I don't know what needs to be done
to reinitialize the list of plugins.

Comment 32

[Brian Ryner (not reading)]

Actually, nevermind, I should have read the comments closer.

Comment 33

[Peter Lubczynski]

Alrighty, a possible plan of action is to add a call to
nsIPluginManger::ReloadPlugins(0) inside nsPluginHostImp::Observe when we get
the right "session-logout" notification. It would actually probably be better to
set a flag at this point and then later when ::LoadPlugins is called, do a
Reload instead.

Comment 34

[Brian Ryner (not reading)]

Attachment: patch for testing

This reloads the plugin list when the last window is closed.  It's kind of a
toss-up whether we want to do it here or when the first window is opened, but a
notification already exists for closing the last window, so this seemed more
convenient.

Comment 35

[Brian Ryner (not reading)]

Attachment: better patch

Took peter's suggestion of setting a flag during session-logout notification.

Comment 36

[Peter Lubczynski]

Comment on attachment 81608 [details] [diff] [review]
better patch

Looks pretty good, except is |ReloadPlugins| ever being called? The check for
|mPluginsLoaded| could possibly be preventing that.

Comment 37

[Brian Ryner (not reading)]

Oh, right, I was thinking about that backwards.  new patch coming up.

Comment 38

[Brian Ryner (not reading)]

Attachment: final patch

Comment 39

[av (gone)]

Comment on attachment 81623 [details] [diff] [review]
final patch

r=av

Comment 40

[Brian Ryner (not reading)]

Peter, do you think you could take care of getting sr= for this patch, checking
it in on the trunk, and requesting branch checkin?

Comment 41

[Peter Lubczynski]

Sure, I'll take it. Thanks for your help!

Comment 42

[Peter Lubczynski]

Marking this bug as a dup of bug 133282 because that bug made this fix obsolete.
In that bug, we will now dynamically call plugins.refresh(0) anytime we "need"
to so, for example, plugins will be automatically found after quick launch is used.

*** This bug has been marked as a duplicate of 133282 ***

Comment 43

[Grace Bush]

verified