Closed Bug 368587 Opened 14 years ago Closed 14 years ago

avoid the second UAC prompt for helper.exe on software update by launching it directly from the elevated updater.exe process

Categories

(Toolkit :: Application Update, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9alpha7

People

(Reporter: moco, Assigned: robert.strong.bugs)

References

Details

(Keywords: verified1.8.1.8)

Attachments

(4 files, 7 obsolete files)

avoid the second UAC prompt (for firefoxHelper.exe) on software update by launching it directly from the elevated updater.exe process

see bug #368353 for details about firefoxHelper.exe.

after robert lands all his code for vista support to trunk/MOZILLA_1_8_BRANCH, here's what will happen on software update on windows (all platforms):

1) firefox.exe downloads the .mar
2) firefox.exe will launch updater.exe (and ask for elevated privs on vista), and will exit firefox.exe
3) updater.exe will apply the mar and start up firefox.exe (but will not elevate firefox.exe)
4) firefox.exe will execute the code in nsUpdateService.js which will execute the code in nsPostUpdateWin.js (which will do some work on the log files), which will in turn call the code which will launch firefoxHelper.exe (and ask for elevated privs on vista, see bug 354226 for more on this step.)  firefoxHelper.exe will finish the work on the log files, and modify the registry.

as you can see, steps 2 and 4 will result in UAC prompts.

we could elimate the second one, and remove nsPostUpdateWin.js if we ported code that works on the logs from nsPostUpdateWin.js to the installer code (remember, firefoxHelper.exe is an NSIS based application)

if we did that, we could then have this:

1) firefox.exe downloads the .mar
2) firefox.exe will launch updater.exe (and ask for elevated privs on vista), and will exit firefox.exe
3) updater.exe will apply the mar, launch firefoxHelper.exe, and start up firefox.exe (but will not elevate firefox.exe)

we'd remove nsPostUpdateWin.js, and we'd remove the code in nsUpdateService.js to launch it.  We still need to be able to launch firefoxHelper.exe (and elevate it) from inside firefox.exe, as we need it when we set the application as default.  but, we could also simplify the code that launches firefoxHelper.exe, as we only need the "firefoxHelper.exe /fixReg" scenario (we could remove the "firefoxHelper.exe /postupdate /uninstalllog=%s" scenario.)

this doesn;t block 2.0.0.2, but it something that both robert and I would like to fix.

note to mscott:  all this goes for tbird as well, with thunderbirdHelper.exe.
nominating just to get this on the radar. jay and triage team can move it to wanted for next release if they deem it appropriate.
Flags: blocking1.8.1.2?
seth:  you said that this doesn't block 2.0.0.2, but is this something you guys are trying to get in for this release?   if we can fit this in, it will be nice to have, but considering everything else happening right now, we can push this out to the next release.
Flags: wanted1.8.1.x+
in my opinion, this shouldn't block 2.0.0.2.

it is something we'd to fix in a future release, to improve the user experience and simplify the code.  (or, to be fair, move code from firefox to the installer)
Flags: blocking1.8.1.3?
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2-
A little explanation... to accomplish this we are going to move the update and old xpinstall uninstall log parsing for adding new entries for the uninstaller into the NSIS code. The NSIS code is much less efficient at parsing these logs and there is the possibility of there being numerous old xpinstall uninstall logs - I had an install that had over 100 for example. Since the NSIS code can take minutes to accomplish this I would like to hold off on this until 3.0 and at that time only parse a couple of the old xpinstall uninstall logs and the update log. The end result would be that some people that upgrade via software update from 1.5.0.x to 3.0 might not have all of the app's files removed.
Blocks: 352420
If it didn't block 2.0.0.2 it probably shouldn't block any future point releases. If you make this better we'll consider approving a trunk-tested patch, though.
Flags: blocking1.8.1.4?
Attached patch work in progress rev1 (obsolete) — Splinter Review
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
still to do:
1. update the uninstall.log with the current update and old xpinstall logs.
2. audit the PostUpdate actions performed by NSIS (e.g. that we can check reg write failure and update HKCU accordingly when we have a manifest - see bug 370457 except with execution level asInvoker, etc.).
3. verify the log used to display update history is located in the app dir at the end of the update so it is available to all users of the system.
Flags: blocking-firefox3?
Summary: avoid the second UAC prompt (for firefoxHelper.exe/thunderbirdHelper.exe) on software update by launching it directly from the elevated updater.exe process → avoid the second UAC prompt for helper.exe on software update by launching it directly from the elevated updater.exe process
Target Milestone: --- → Firefox 3 M7
(In reply to comment #7)
> still to do:
>...
> 3. verify the log used to display update history is located in the app dir at
> the end of the update so it is available to all users of the system.
We use the one located in the user's profile. Submitted bug 388200 to cover this separately.
Attached patch NSIS patch wip rev1 (obsolete) — Splinter Review
Attached patch NSIS patch wip rev2 -w (obsolete) — Splinter Review
Attachment #272743 - Attachment is obsolete: true
Attachment #272987 - Attachment description: NSIS patch wip rev2 → NSIS patch wip rev2 -w
Attached patch patch -w rev1 (obsolete) — Splinter Review
Attachment #272348 - Attachment is obsolete: true
Attachment #272987 - Attachment is obsolete: true
Attachment #273020 - Flags: review?(sspitzer)
Attached patch patch (obsolete) — Splinter Review
Attachment #273020 - Flags: superreview?(benjamin)
Comment on attachment 273020 [details] [diff] [review]
patch -w rev1

Found a build problem with this patch... will re-submit after I get that worked out
Attachment #273020 - Flags: superreview?(benjamin)
Attachment #273020 - Flags: review?(sspitzer)
Attached patch patch -w rev2 (obsolete) — Splinter Review
The previous patches had a couple of extra spaces in a Makefile.in... other than that this is the same.
Attachment #273020 - Attachment is obsolete: true
Attachment #273045 - Flags: review?(sspitzer)
Attached patch patch rev2 (obsolete) — Splinter Review
Attachment #273023 - Attachment is obsolete: true
Comment on attachment 273045 [details] [diff] [review]
patch -w rev2

ignore the change to shared.nsh... I've added that to the patch in bug 388932
One thing (e.g. oops) that came to mind this weekend is that the new updater.ini is the one I should be reading... besides that I believe the process flow is correct.
Comment on attachment 273045 [details] [diff] [review]
patch -w rev2

r=sspitzer

can you log a spin off bug about the code we should remove once bug 386760 is fixed?
Attachment #273045 - Flags: review?(sspitzer) → review+
Flags: blocking-firefox3? → blocking-firefox3+
Whiteboard: [patch has review][checkin needed?]
Comment on attachment 273045 [details] [diff] [review]
patch -w rev2

clearing review request, I believe robert is working on a new version that addresses the ini file location issue.
Attachment #273045 - Flags: review+
Whiteboard: [patch has review][checkin needed?]
Attached patch patch -w rev3Splinter Review
Benjamin, this has a change from attachment 273045 [details] [diff] [review] which sspitzer r+'d earlier in that this uses the updated updater.ini in the app dir. I'm unsure if this change will adversely affect xulrunner apps and whether this is the best way to append the updater_append.ini to updater.ini.
Attachment #273045 - Attachment is obsolete: true
Attachment #273048 - Attachment is obsolete: true
Attachment #273752 - Flags: review?(benjamin)
Missing the freeze, moving out.
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Comment on attachment 273752 [details] [diff] [review]
patch -w rev3

I don't know how this would affect any XR apps. The only one I know has update working is Songbird, so we should ask bent.
Attachment #273752 - Flags: review?(benjamin) → review+
Ben, heads up about changes to updater.exe and how we will make the post update changes on Win32. I think this should be flexible enough for your needs and if it isn't please file a new bug and cc me.
Thanks Rob. Sounds like this should work well.
Landed on Trunk with a verbal a=mconnor

Checking in mozilla/toolkit/mozapps/update/src/Makefile.in;
/cvsroot/mozilla/toolkit/mozapps/update/src/Makefile.in,v  <--  Makefile.in
new revision: 1.13; previous revision: 1.12
done
Checking in mozilla/toolkit/mozapps/update/src/nsPostUpdateWin.js;
/cvsroot/mozilla/toolkit/mozapps/update/src/nsPostUpdateWin.js,v  <--  nsPostUpdateWin.js
new revision: 1.16; previous revision: 1.15
done
Checking in mozilla/toolkit/mozapps/update/src/updater/updater.cpp;
/cvsroot/mozilla/toolkit/mozapps/update/src/updater/updater.cpp,v  <--  updater.cpp
new revision: 1.28; previous revision: 1.27
done
Checking in mozilla/browser/locales/Makefile.in;
/cvsroot/mozilla/browser/locales/Makefile.in,v  <--  Makefile.in
new revision: 1.52; previous revision: 1.51
done
RCS file: /cvsroot/mozilla/browser/installer/windows/nsis/updater_append.ini,v
done
Checking in mozilla/browser/installer/windows/nsis/updater_append.ini;
/cvsroot/mozilla/browser/installer/windows/nsis/updater_append.ini,v  <--  updater_append.ini
initial revision: 1.1
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh,v  <--  common.nsh
new revision: 1.18; previous revision: 1.17
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Note: you will still get the second UAC prompt until you receive your second Software Update after this patch has landed. From that point forward you should only get one UAC prompt. I'll provide steps for verifying this is fixed later.
Target Milestone: Firefox 3 M8 → Firefox 3 M7
Depends on: 390334
To verify (must be running Vista):

1. Install the previous nightly build using the installer.
2. Copy the uninstall.log from the appdir's uninstall directory to your desktop.
3. Open the uninstall.log and remove the following:
   File: \freebl3.chk
   File: \softokn3.chk
4. Copy the uninstall.log you just modified to the appdir's uninstall directory and replace the existing uninstall.log.
4. Open regedit and navigate to
   HKEY_LOCAL_MACHINE\Software\Microsoft\Windows\CurrentVersion\Uninstall\Minefield (3.0a7pre)
5. Change the value of Comments to Minefield bogus
6. Run Firefox, check for updates, install the update, and click restart.
7. You should receive a UAC prompt to elevate updater.exe - click allow.

After the previous steps have been completed the following should be observed.
1. A UAC prompt for helper.exe was NOT displayed.
2. The uninstall.log in the appdir's uninstall directory should have the following added:
   File: \freebl3.chk
   File: \softokn3.chk
3. The value of Comments under:
   HKEY_LOCAL_MACHINE\Software\Microsoft\Windows\CurrentVersion\Uninstall\Minefield (3.0a7pre)
   should be Minefield
Got my numbering wrong above but I'm sure you get the jist of it! ;)
verified fixed on trunk with  Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a7pre) Gecko/200707310505 Minefield/3.0a7pre as the old build and Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a7pre) Gecko/200708020404 Minefield/3.0a7pre as the patched build.

I followed the steps to reproduce from rstrong and can confirm that :
- no second UAC prompt for helper.exe was displayed during update (only the one for updater.exe)

- File: \freebl3.chk and File: \softokn3.chk was added to the uninstall.log after update (was deleted on the uninstall.log of 200707310505)

and HKEY_LOCAL_MACHINE\Software\Microsoft\Windows\CurrentVersion\Uninstall\Minefield (3.0a7pre) was renamed after the Update to Minefield (after renaming in 200707310505 to Minefield Bogus)

-> changing status to verified fixed
Status: RESOLVED → VERIFIED
Tomcat, thanks and you rock!
Attachment #273752 - Flags: approval1.8.1.7?
Comment on attachment 273752 [details] [diff] [review]
patch -w rev3

approved for 1.8.1.7, a=dveditz for release-drivers
Attachment #273752 - Flags: approval1.8.1.7? → approval1.8.1.7+
Attachment #274513 - Flags: approval1.8.1.7+
Checked in to MOZILLA_1_8_BRANCH

note: I decided not to bother checking in the change to mozilla/toolkit/mozapps/update/src/Makefile.in since it is just a correctness fix not necessary to fix this bug.

Checking in mozilla/toolkit/mozapps/update/src/nsPostUpdateWin.js;
/cvsroot/mozilla/toolkit/mozapps/update/src/nsPostUpdateWin.js,v  <--  nsPostUpdateWin.js
new revision: 1.2.2.10; previous revision: 1.2.2.9
done
Checking in mozilla/toolkit/mozapps/update/src/updater/updater.cpp;
/cvsroot/mozilla/toolkit/mozapps/update/src/updater/updater.cpp,v  <--  updater.cpp
new revision: 1.11.4.14; previous revision: 1.11.4.13
done
Checking in mozilla/browser/locales/Makefile.in;
/cvsroot/mozilla/browser/locales/Makefile.in,v  <--  Makefile.in
new revision: 1.25.2.19; previous revision: 1.25.2.18
done
Checking in mozilla/browser/installer/windows/nsis/uninstaller.nsi;
/cvsroot/mozilla/browser/installer/windows/nsis/uninstaller.nsi,v  <--  uninstaller.nsi
new revision: 1.1.2.6; previous revision: 1.1.2.5
done
Checking in mozilla/browser/installer/windows/nsis/updater_append.ini;
/cvsroot/mozilla/browser/installer/windows/nsis/updater_append.ini,v  <--  updater_append.ini
new revision: 1.1.2.1; previous revision: 1.1
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh,v  <--  common.nsh
new revision: 1.2.2.18; previous revision: 1.2.2.17
done
Keywords: fixed1.8.1.7
note: the second UAC prompt will still be seen during the next software update and subsequent software updates after the next one should not display the second UAC prompt. See comment #29 for information on how to verify.
verified fixed 1.8.1.7 using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.7pre) Gecko/2007090703 BonEcho/2.0.0.7pre and per final update to Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.7pre) Gecko/2007091103 BonEcho/2.0.0.7pre

Same results as in comment #31 with the steps to verify:

-Registry was renamed to Bon Echo after changed to Bon Echo Bogus and File: \freebl3.chk and File: \softokn3.chk was added to the uninstall.log
after update

also no second UAC prompt after the final update from 2007-09-10 build to the 2007-09-11 build see comment #36

adding verified keyword
Litmus Triage Team: Tomcat will cover the Litmus test case for this one.
Depends on: 401608
https://litmus.mozilla.org/show_test.cgi?id=5086 has been added to the Vista specific test suite.
Flags: in-litmus? → in-litmus+
Product: Firefox → Toolkit
Blocks: 459436
You need to log in before you can comment on or make changes to this bug.