The plugin finder service, or PFS, (which includes "seamless" installation of Flash) can't install any plugins on Vista. Installing XPIs no longer works in Firefox 3 for at least Vista. Some of that is discussed in bug 352762. However, installerLocation and installerHash, when used, produce a bad user experience for users (see bug 419928 attachment 314482 [details]). The user does not know what is happening, and the PFS dialog actually tells them that the install failed even though the external installer from installerLocation (after checking installerHash) has been executed. Expected: 1. user gets prompted by PFS to install 2. user chooses to install 3. user installs, PFS tells them it was successful and they can use their plugin Actual: 1. user gets prompted by PFS to install 2. user chooses to install 3. PFS says the installation failed 4. external installer is still running

Jennifer, maybe you can take some time to help us build a better experience here? It feels like the full-bore modal dialog is probably something we might want to investigate ditching as well.

Just to note bug 430853 is open about the kind of whole PFS experience, I certainly want to see changes there though that kind of large scale change is not going to happen in a 3.0 timeframe obviously.

So - to make things clear - in order to get things working with installerLocation and installerHash: 1) the user experience needs to be redone for PFS - bug 430853 2) the client code needs to change to follow the revised workflow - this bug

I don't think bug 430853 really blocks this bug, so they are different and should be prioritized independently. This bug is the real issue surrounding plugin installation in the current UI. If that UI changes, this will still be a problem.

I have a patch in progress that fixes this based on sdwilsh's work in Bug 429988. It launches the installer via nsIProcess on a separate thread so nsIProcess I'm making it thread safe. While looking over nsIProcess I see that Kill is not available at all for Windows and only works on Mac OS X when Run has command line args. For now I'd like to make Kill return NS_ERROR_NOT_IMPLEMENTED since it is to say the least hit or miss.

Great news. Let me know if there's anything I can do to help/test.

Is this just for branch? I understand that there are alternate plans afoot for pfs for 3.1

Is there a bug?

I do not believe so however mconnor has a wealth of knowledge I am positive

Morgamic - do you think it's worth fixing the UI anyway once the external installer problem is fixed?

(In reply to comment #10) > Morgamic - do you think it's worth fixing the UI anyway once the external > installer problem is fixed? There really isn't a problem with the external installer... it has to do with that we try to display success or failure based on the external installer's exit code and we just launch it without waiting on the installer to finish and then checking the exit code.

@boriss - I think the UI fix is on a different timeline, and you should talk to mconnor about that. I believe that either way this mechanism should be addressed -- it will have to support any future workflow PFS might have in the future that deals with external installers.

Rob - any progress on this one?

I've been busy with a couple of App Update bugs and haven't made much progress on this as of late. One issue is that the standard methods for installing a plugin on at least Mac OS X e.g. .dmg's (not sure about Linux) don't provide a result code after the install is complete.

Since mac+linux don't support external installers could we still support xpi's on those platforms to ensure a good user experience?

(In reply to comment #15) > Since mac+linux don't support external installers could we still support xpi's > on those platforms to ensure a good user experience? We have completely removed from the platform support for the old style install.js xpi's that plugin vendors used to use. It also wouldn't really work, almost certainly on linux and probably mac for the same reasons as vista, the user has no write access to the install dir. We can support new-style xpi based installation for plugins on all platforms (https://developer.mozilla.org/en/Deploying_a_Plugin_as_an_Extension) but it only allows installing into the profile folder. We might be able to launch pkg's on mac which I think is the preferred delivery format for plugins there (ignoring a surrounding dmg). I'm surprised it isn't possible for an external installer to work on mac and linux though.

Rob I forget if you know this code or not? Anyway this is an entry level patch that fixes all the issues I could find with installer location use in plugins. It is applicable to the 3.0.x branch as well as trunk and includes a test to verify that we can download and at least launch an installer. The main issue is an assumption that we need a proper filename in the url. I've removed this by just using a fixed filename of setup or setup.exe depending on the platform. There shouldn't be a need to use a particular filename per installer I think. The UI was never fixed correctly to handle installers so it displays an error when one is used, this is fixed here. I've added calling the plugin manager to do a reload since this doesn't happen automatically in some cases. Also fixed the refreshBrowser method to actually get the browser. Simplest way to test is to set pfs.datasource.url to http://people.mozilla.com/~dtownsend/testcases/bug435788/pfs.rdf and then visit http://people.mozilla.com/~dtownsend/testcases/bug435788/pfs.html on a windows machine without quicktime installed.

Comment on attachment 368568 [details] [diff] [review] stage 1 rev 1 >diff --git a/toolkit/mozapps/plugins/tests/TestInstaller.cpp b/toolkit/mozapps/plugins/tests/TestInstaller.cpp >new file mode 100644 >--- /dev/null >+++ b/toolkit/mozapps/plugins/tests/TestInstaller.cpp >@@ -0,0 +1,13 @@ >+#if defined(XP_WIN) && !defined(__GNUC__) >+#include <windows.h> >+ >+int WINAPI wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) >+{ >+ return 0; >+} >+#else >+int main(int argc, char** argv) >+{ >+ return 0; >+} >+#endif Might be good to test both success and failure. I suspect that this will cause elevation on Vista which we can't test for since the exe name will have "setup" in it per the resultFile.createUnique above as well as this file if launched directly since it has "Install" in it... I'll check this. Adding a manifest to this file with requestedExecutionLevel level="asInvoker" would resolve this. I'll check the rest after I have a chance to compile the patch.

(In reply to comment #18) > (From update of attachment 368568 [details] [diff] [review]) > >diff --git a/toolkit/mozapps/plugins/tests/TestInstaller.cpp b/toolkit/mozapps/plugins/tests/TestInstaller.cpp > >new file mode 100644 > >--- /dev/null > >+++ b/toolkit/mozapps/plugins/tests/TestInstaller.cpp > >@@ -0,0 +1,13 @@ > >+#if defined(XP_WIN) && !defined(__GNUC__) > >+#include <windows.h> > >+ > >+int WINAPI wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) > >+{ > >+ return 0; > >+} > >+#else > >+int main(int argc, char** argv) > >+{ > >+ return 0; > >+} > >+#endif > Might be good to test both success and failure. At least on 1.9.0.x branch which is where I want this patch to end up we can't get the exit code for the process, all we can do is assume it completed. A version targeted at 1.9.1 is in progress which will include that. I hadn't realised vista used filenames to determine UAC, but we could just use a different name without issue I think.

It uses the filename if the binary doesn't have a manifest so it will require elevation for installers that haven't been updated for Vista yet.

(In reply to comment #20) > It uses the filename if the binary doesn't have a manifest so it will require > elevation for installers that haven't been updated for Vista yet. just throwing it out there, but maybe that might be a good thing? If the installer hasn't been updated and it is still trying to do a global install then it'll need UAC to work right. Though that would mean we need to add the manifest to the test installer to stop tests failing there.

(In reply to comment #21) > (In reply to comment #20) > > It uses the filename if the binary doesn't have a manifest so it will require > > elevation for installers that haven't been updated for Vista yet. > > just throwing it out there, but maybe that might be a good thing? If the > installer hasn't been updated and it is still trying to do a global install > then it'll need UAC to work right. > > Though that would mean we need to add the manifest to the test installer to > stop tests failing there. I'm not so sure. If they want to provide elevation they can provide a manifest that asks for elevation and most installers do this when necessary. I'd suggest using the same filename so the installer itself was able to control this.

(In reply to comment #22) > (In reply to comment #21) > > (In reply to comment #20) > > > It uses the filename if the binary doesn't have a manifest so it will require > > > elevation for installers that haven't been updated for Vista yet. > > > > just throwing it out there, but maybe that might be a good thing? If the > > installer hasn't been updated and it is still trying to do a global install > > then it'll need UAC to work right. > > > > Though that would mean we need to add the manifest to the test installer to > > stop tests failing there. > I'm not so sure. If they want to provide elevation they can provide a manifest > that asks for elevation and most installers do this when necessary. I'd suggest > using the same filename so the installer itself was able to control this. Ok, so we either use a filename that doesn't trigger elevation, or we somehow get a filename from the downloaded installer. The problem with that is it places restrictions on the url that installer vendors can give us. The biggest problem with installing the java plugin was that Sun gave us the url http://java.com/firefoxjre_exe which gives us a bogus filename. I don't necessarily mind just going back to using the filename part of the url (maybe be a bit more sensible and stick .exe on the end if it isn't there on windows or something), we just need to make sure everyone is aware of the restrictions that entails.

Comment on attachment 368568 [details] [diff] [review] stage 1 rev 1 Looks good... just a couple of nits besides figuring out the naming which is the only reason I am minusing. >diff --git a/toolkit/mozapps/plugins/Makefile.in b/toolkit/mozapps/plugins/Makefile.in >--- a/toolkit/mozapps/plugins/Makefile.in >+++ b/toolkit/mozapps/plugins/Makefile.in >@@ -39,10 +39,19 @@ DEPTH = ../../.. > topsrcdir = @top_srcdir@ > srcdir = @srcdir@ > VPATH = @srcdir@ > > include $(DEPTH)/config/autoconf.mk > > EXTRA_COMPONENTS = pluginGlue.js > >+ifdef ENABLE_TESTS >+# OSX will display a security warning when running the downloaded installer >+ifneq (Darwin,$(OS_TARGET)) >+# Linux fails to run the downloaded installer Could you improve this comment? Doesn't it run when the user has permissions? >diff --git a/toolkit/mozapps/plugins/content/pluginInstallerService.js b/toolkit/mozapps/plugins/content/pluginInstallerService.js >--- a/toolkit/mozapps/plugins/content/pluginInstallerService.js >+++ b/toolkit/mozapps/plugins/content/pluginInstallerService.js >@@ -80,27 +80,30 @@ InstallerObserver.prototype = { > _init: function() > { > try { > var ios = Components.classes["@mozilla.org/network/io-service;1"]. > getService(Components.interfaces.nsIIOService); > var uri = ios.newURI(this._plugin.InstallerLocation, null, null); > uri.QueryInterface(Components.interfaces.nsIURL); > >- var leafName = uri.fileName; >- if (leafName.indexOf('.') == -1) >- throw "Filename needs to contain a dot for platform-native launching to work correctly."; >+ // Use a local filename appropriate for the OS >+ var leafName = "setup"; >+ if (Components.classes["@mozilla.org/xre/app-info;1"] >+ .getService(Components.interfaces.nsIXULRuntime) >+ .OS == "WINNT") >+ leafName += ".exe"; >diff --git a/toolkit/mozapps/plugins/content/pluginInstallerWizard.js b/toolkit/mozapps/plugins/content/pluginInstallerWizard.js >--- a/toolkit/mozapps/plugins/content/pluginInstallerWizard.js >+++ b/toolkit/mozapps/plugins/content/pluginInstallerWizard.js >... >@@ -664,19 +665,31 @@ function wizardFinish(){ > .getService(nsIAppStartup); > appStartup.quit(nsIAppStartup.eAttemptQuit | nsIAppStartup.eRestart); > return true; > } > } > > // don't refresh if no plugins were found or installed > if ((gPluginInstaller.mSuccessfullPluginInstallation > 0) && >- (gPluginInstaller.mPluginInfoArray.length != 0) && >- gPluginInstaller.mBrowser) { >- // notify listeners that a plugin is installed, >- // so that they can reset the UI and update the browser. >- var event = document.createEvent("Events"); >- event.initEvent("NewPluginInstalled", true, true); >- gPluginInstaller.mBrowser.dispatchEvent(event); >+ (gPluginInstaller.mPluginInfoArray.length != 0)) { nit: indentation >diff --git a/toolkit/mozapps/plugins/tests/pfs_bug435788_1.rdf b/toolkit/mozapps/plugins/tests/pfs_bug435788_1.rdf >new file mode 100644 >--- /dev/null >+++ b/toolkit/mozapps/plugins/tests/pfs_bug435788_1.rdf >@@ -0,0 +1,31 @@ >... >+ >+</RDF:RDF> >\ No newline at end of file Please add a newline

btw: I did run the mochitest and it did request elevation

(In reply to comment #24) > >+ifdef ENABLE_TESTS > >+# OSX will display a security warning when running the downloaded installer > >+ifneq (Darwin,$(OS_TARGET)) > >+# Linux fails to run the downloaded installer > Could you improve this comment? Doesn't it run when the user has permissions? The API nsILocalFile.launch basically isn't made to launch executables. It seems to work ok on Windows and OSX, but on Linux it really can only open documents. Executables have no default handler registered in gnome and so it fails to run.

Fixes the nits. What I've decided to do for the filename is go back to using the fileName part of the uri, however on windows if the filename doesn't end with ".exe" then that is appended to make it into something that will definitely execute. This should allow plugin vendors to use an installer filename of their choice but also allow them to use any URL that they wish.

This is a patch for the next stage of improvements. Unlike the stage 1 patch this will only be targetted at trunk and 1.9.1 branches. As such it tidies up more issues that aren't really critical for fixing on the 1.9.0.x branch. This patch will apply on top of the stage 1 patch and the patches in bug 480427. Most of the issues are around handling points where the plugin search or install fails due to problems with the downloaded datasource however I've also added some extra points where errors are logged to the error console in exceptional circumstances. This should help us diagnose any errors users are seeing. It also adds support for detecting when the installer has finished its work and it makes the progress bar undetermined for that period. There's also a bunch of undeclared variables that I've tidied up here as well as a lot more tests.

Cleans up the diffs a little

Cleans up the diffs a little

Comment on attachment 369096 [details] [diff] [review] stage 2 rev 1 Looks good but I haven't test this version

Comment on attachment 369096 [details] [diff] [review] stage 2 rev 1 bah... approved the wrong one. I'm reviewing this right now

Comment on attachment 369096 [details] [diff] [review] stage 2 rev 1 >diff --git a/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js b/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js >--- a/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js >+++ b/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js >@@ -82,17 +82,16 @@ nsRDFItemUpdater.prototype = { > onDatasourceLoaded: function pfs_onDatasourceLoaded (aDatasource, aPluginRequestItem){ nit: if you don't mind while you are cleaning this up add a space between ){ for the functions including the nsIRDFXMLSinkObserver functions. > var container = Components.classes["@mozilla.org/rdf/container;1"]. > createInstance(Components.interfaces.nsIRDFContainer); nit: if you don't mind this file aligns on . > var resultRes = this._rdfService.GetResource("urn:mozilla:plugin-results:" + aPluginRequestItem.mimetype); > var pluginList = aDatasource.GetTarget(resultRes, this._rdfService.GetResource(PFS_NS+"plugins"), true); > > var pluginInfo = null; > >- container = Components.classes["@mozilla.org/rdf/container;1"].createInstance(Components.interfaces.nsIRDFContainer); > try { > container.Init(aDatasource, pluginList); > > var children = container.GetElements(); > var target; > > // get the first item > var child = children.getNext(); >@@ -134,26 +133,31 @@ nsRDFItemUpdater.prototype = { > XPIHash: getPFSValueFromRDF("XPIHash"), > InstallerShowsUI: getPFSValueFromRDF("InstallerShowsUI"), > manualInstallationURL: getPFSValueFromRDF("manualInstallationURL"), > requestedMimetype: getPFSValueFromRDF("requestedMimetype"), > licenseURL: getPFSValueFromRDF("licenseURL"), > needsRestart: getPFSValueFromRDF("needsRestart") > }; > } >- catch (ex){} >+ catch (ex){ >+ Components.utils.reportError(ex); >+ } > } >- catch (ex){} >+ catch (ex){ nit: add a space between ){ for the two catches in this file

With both patches applied I get the following $ TEST_PATH=toolkit/mozapps/plugins/tests/browser_bug435788.js make -C /c/moz/_1_mozilla-central/ff-opt/ mochitest INFO | (automation.py) | Application ran for: 0:00:03.026000 SUCCESS: The process with PID 5188 has been terminated. WARNING refcount logging is off, so leaks can't be detected! mochitest-browser-chrome failed: TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/plugins /tests/browser_bug435788.js | Should have been a successful install - Got Failed , expected Installed TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/plugins /tests/browser_bug435788.js | Should have been a failed install - Got Failed, ex pected Installed make: *** [mochitest-browser-chrome] Error 1

(In reply to comment #34) > With both patches applied I get the following Did you apply the patches in bug 480427 too? If so I'll have to see if I can try to test this on Vista or Windows 7 tomorrow.

I cleaned up my tree and forgot to reapply them.

The strings that come to mind that will make Vista request elevation that you might be tempted to use are install, update, and setup. If the test binary names have any of these they will need a manifest with requestedExecutionLevel level="asInvoker" or you can change the names.

btw: the string just has to be a substring of the file name

Also getting an error about not finding the MOZCRT19.dll

(In reply to comment #37) > The strings that come to mind that will make Vista request elevation that you > might be tempted to use are install, update, and setup. If the test binary > names have any of these they will need a manifest with requestedExecutionLevel > level="asInvoker" or you can change the names. Damn, so TestInstaller will be triggering it. Ok I can rename that. (In reply to comment #39) > Created an attachment (id=369160) [details] > error screenshot > > Also getting an error about not finding the MOZCRT19.dll Ugh, I guess I'm not building with jemalloc enabled here. I'll have to see if I can work out a way around that.

(In reply to comment #40) >... > (In reply to comment #39) > > Created an attachment (id=369160) [details] [details] > > error screenshot > > > > Also getting an error about not finding the MOZCRT19.dll > > Ugh, I guess I'm not building with jemalloc enabled here. I'll have to see if I > can work out a way around that. Add the following to the plugins/tests/Makefile.in USE_STATIC_LIBS = 1

Why use int WINAPI wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) instead of using int main(int argc, char** argv) for both?

(In reply to comment #42) > Why use > int WINAPI wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) > > instead of using > > int main(int argc, char** argv) > > for both? I was at one point seeing the console window appear when running the tests. It's possible changes I made since then make it unnecessary. I guess it's also true that it doesn't matter much for the tests if the console appears and disappears a few times.

btw: I believe the USE_STATIC_LIBS = 1 should only be necessary for Windows

Comment on attachment 369096 [details] [diff] [review] stage 2 rev 1 Let's go with new patches on these two for the existing issues. The remaining content/*.js changes look good and all that's left are the tests

I've changed the name of the installer to GoodPlugin and BadPlugin so we shouldn't be hitting UAC and indeed I have tested this on a windows 7 install and it gets through the tests fine.

Now that bug 480427 has landed nothing else is required for these patches to apply to trunk. Cleaned up the JS style wherever I could see a problem. I removed the winmain usage and it does give a console window for a few moments but that isn't really a problem I think.

Comment on attachment 369276 [details] [diff] [review] stage 1 rev 3 >diff --git a/toolkit/mozapps/plugins/content/pluginInstallerService.js b/toolkit/mozapps/plugins/content/pluginInstallerService.js >--- a/toolkit/mozapps/plugins/content/pluginInstallerService.js >+++ b/toolkit/mozapps/plugins/content/pluginInstallerService.js >@@ -80,27 +80,31 @@ InstallerObserver.prototype = { > _init: function() > { > try { > var ios = Components.classes["@mozilla.org/network/io-service;1"]. > getService(Components.interfaces.nsIIOService); > var uri = ios.newURI(this._plugin.InstallerLocation, null, null); > uri.QueryInterface(Components.interfaces.nsIURL); > >+ // Use a local filename appropriate for the OS > var leafName = uri.fileName; >- if (leafName.indexOf('.') == -1) >- throw "Filename needs to contain a dot for platform-native launching to work correctly."; >+ var os = Components.classes["@mozilla.org/xre/app-info;1"] >+ .getService(Components.interfaces.nsIXULRuntime) >+ .OS; >+ if (os == "WINNT" && leafName.substr(-4) != ".exe") >+ leafName += ".exe"; This should also check for .msi so it doesn't append .exe to msi's >diff --git a/toolkit/mozapps/plugins/tests/GoodPlugin.cpp b/toolkit/mozapps/plugins/tests/GoodPlugin.cpp >new file mode 100644 >--- /dev/null >+++ b/toolkit/mozapps/plugins/tests/GoodPlugin.cpp >@@ -0,0 +1,4 @@ >+int main(int argc, char** argv) >+{ >+ return 0; >+} >diff --git a/toolkit/mozapps/plugins/tests/Makefile.in b/toolkit/mozapps/plugins/tests/Makefile.in >new file mode 100644 >--- /dev/null >+++ b/toolkit/mozapps/plugins/tests/Makefile.in >@@ -0,0 +1,72 @@ >... >+ >+DEPTH = ../../../.. >+topsrcdir = @top_srcdir@ >+srcdir = @srcdir@ >+VPATH = @srcdir@ >+relativesrcdir = toolkit/mozapps/plugins/tests >+TESTROOT = $(DEPTH)/_tests/testing/mochitest/browser/$(relativesrcdir) >+USE_STATIC_LIBS = 1 >+ >+include $(DEPTH)/config/autoconf.mk nit: most Makefiles in toolkit have an empty line following VPATH followed by the include for autoconf.mk followed by a space, etc. With those fixed r=me I'll also verify this with a debug build as soon as it finished compiling

(In reply to comment #48) >... > nit: most Makefiles in toolkit have an empty line following VPATH followed by > the include for autoconf.mk followed by a space, etc. s/followed by a space/followed by an empty line/

(In reply to comment #48) >... > I'll also verify this with a debug build as soon as it finished compiling All tests look good on debug and non-debug builds.

Comment on attachment 369278 [details] [diff] [review] stage 2 rev 2 [checked in] I verified these tests as well with debug and non-debug on Vista

(In reply to comment #48) > (From update of attachment 369276 [details] [diff] [review]) > >diff --git a/toolkit/mozapps/plugins/content/pluginInstallerService.js b/toolkit/mozapps/plugins/content/pluginInstallerService.js > >--- a/toolkit/mozapps/plugins/content/pluginInstallerService.js > >+++ b/toolkit/mozapps/plugins/content/pluginInstallerService.js > >@@ -80,27 +80,31 @@ InstallerObserver.prototype = { > > _init: function() > > { > > try { > > var ios = Components.classes["@mozilla.org/network/io-service;1"]. > > getService(Components.interfaces.nsIIOService); > > var uri = ios.newURI(this._plugin.InstallerLocation, null, null); > > uri.QueryInterface(Components.interfaces.nsIURL); > > > >+ // Use a local filename appropriate for the OS > > var leafName = uri.fileName; > >- if (leafName.indexOf('.') == -1) > >- throw "Filename needs to contain a dot for platform-native launching to work correctly."; > >+ var os = Components.classes["@mozilla.org/xre/app-info;1"] > >+ .getService(Components.interfaces.nsIXULRuntime) > >+ .OS; > >+ if (os == "WINNT" && leafName.substr(-4) != ".exe") > >+ leafName += ".exe"; > This should also check for .msi so it doesn't append .exe to msi's Do you think maybe it might be better to just check if there was a "." in the leafname, and only append .exe if not?

Comment on attachment 369278 [details] [diff] [review] stage 2 rev 2 [checked in] Looks good!

(In reply to comment #52) >... > Do you think maybe it might be better to just check if there was a "." in the > leafname, and only append .exe if not? That sounds like a good idea. The case with the JRE should be an exception and not the rule though handling it would be a good thing.

Carrying across review to just take care of those two changes. Will land this tomorrow.

Comment on attachment 369374 [details] [diff] [review] stage 1 rev 4 [checked in] Checked in stage 1: http://hg.mozilla.org/mozilla-central/rev/8a50fbd3288e

Checked in stage 2: http://hg.mozilla.org/mozilla-central/rev/839fbc4a9e3d

Comment on attachment 369374 [details] [diff] [review] stage 1 rev 4 [checked in] I would like approval to land both of these patches on the 1.9.1 branch. They make it possible to install plugins using installers through the plugin finder service. Both patches come with tests to verify their behaviour.

Comment on attachment 369278 [details] [diff] [review] stage 2 rev 2 [checked in] a191=beltzner

Comment on attachment 369374 [details] [diff] [review] stage 1 rev 4 [checked in] a191=beltzner

Landed stage 1 on branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5a120fca0323

Landed stage 2 on branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/43ae732dbc41

I filed bug 488593 about possible randomness in a test this bug added.

There is also bug 487489 (looks like bug 488593 is a dupe of that) and Bug 487717

Comment on attachment 369374 [details] [diff] [review] stage 1 rev 4 [checked in] This is a small fix that would allow us to start offering plugin installers to 3.0.x users. While technically outside the scope of a security/stability update it is a small patch that only touches the PFS and includes tests to verify its behaviour so I think it would be reasonable to take it on the branch.

Comment on attachment 369374 [details] [diff] [review] stage 1 rev 4 [checked in] Approved for 1.9.0.10, a=dveditz for release-drivers

Checked in stage 1 to 1.9.0.x branch: Checking in browser/base/content/browser.js; /cvsroot/mozilla/browser/base/content/browser.js,v <-- browser.js new revision: 1.1042; previous revision: 1.1041 done Checking in toolkit/mozapps/plugins/Makefile.in; /cvsroot/mozilla/toolkit/mozapps/plugins/Makefile.in,v <-- Makefile.in new revision: 1.2; previous revision: 1.1 done Checking in toolkit/mozapps/plugins/content/pluginInstallerService.js; /cvsroot/mozilla/toolkit/mozapps/plugins/content/pluginInstallerService.js,v <-- pluginInstallerService.js new revision: 1.5; previous revision: 1.4 done Checking in toolkit/mozapps/plugins/content/pluginInstallerWizard.js; /cvsroot/mozilla/toolkit/mozapps/plugins/content/pluginInstallerWizard.js,v <-- pluginInstallerWizard.js new revision: 1.26; previous revision: 1.25 done RCS file: /cvsroot/mozilla/toolkit/mozapps/plugins/tests/GoodPlugin.cpp,v done Checking in toolkit/mozapps/plugins/tests/GoodPlugin.cpp; /cvsroot/mozilla/toolkit/mozapps/plugins/tests/GoodPlugin.cpp,v <-- GoodPlugin.cpp initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/mozapps/plugins/tests/Makefile.in,v done Checking in toolkit/mozapps/plugins/tests/Makefile.in; /cvsroot/mozilla/toolkit/mozapps/plugins/tests/Makefile.in,v <-- Makefile.in initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/mozapps/plugins/tests/browser_bug435788.js,v done Checking in toolkit/mozapps/plugins/tests/browser_bug435788.js; /cvsroot/mozilla/toolkit/mozapps/plugins/tests/browser_bug435788.js,v <-- browser_bug435788.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/mozapps/plugins/tests/pfs_bug435788_1.rdf,v done Checking in toolkit/mozapps/plugins/tests/pfs_bug435788_1.rdf; /cvsroot/mozilla/toolkit/mozapps/plugins/tests/pfs_bug435788_1.rdf,v <-- pfs_bug435788_1.rdf initial revision: 1.1 done

Dave, is this in a 3.0.x version yet? Was it released?

(In reply to comment #68) > Dave, is this in a 3.0.x version yet? Was it released? Should be in 3.0.11 whenever that is released, or nightlies of that should have it.

(In reply to comment #17) > > Simplest way to test is to set pfs.datasource.url to > http://people.mozilla.com/~dtownsend/testcases/bug435788/pfs.rdf and then visit > http://people.mozilla.com/~dtownsend/testcases/bug435788/pfs.html on a windows > machine without quicktime installed. Verified for 1.9.0.11 with the above test on Vista.