Closed Bug 370571 Opened 13 years ago Closed 12 years ago

Ability to install as a standard user on Vista is not available

Categories

(Firefox :: Installer, defect)

x86
Windows Vista
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 3 beta1

People

(Reporter: rstrong, Assigned: rstrong)

References

Details

Attachments

(4 files, 5 obsolete files)

Need to figure out a decent method to allow installation as a standard user on Vista.

7-Zip SEA's always require elevation on Vista unless they have a manifest that states asInvoker. Same goes for NSIS installer but that is neither here nor there for this bug since it inherits from the 7-Zip SEA.

We could add the asInvoker to the 7-Zip SEA, launch another exe that displays ui providing the user a choice to install system wide or for the user, launch another exe if system wide is selected which would display the UAC prompt to request elevation or continue on in the user case, etc.

I believe that MSI installers provide the ability to do either in a much simpler manner.
I'd like to get this for Firefox 3.0. Most PC's that are delivered with Windows will be delivered with Vista going forward so fixing this would most likely be a great win. I have a couple of ideas on how we can accomplish this with NSIS and will work up a proof of concept.
Assignee: nobody → robert.bugzilla
Flags: blocking-firefox3?
Proposed behavior (XP and below behavior will not change):

First check if the installer is running on Vista or above
Vista or above

If not Vista or above the installer behaves the same as it does now.

If Vista or above the installer checks if we are already elevated.

If elevated the installer behaves the same as it does now for a "Everyone" install.

If not elevated the installer checks if both of the following
UAC Enabled
User is a member of the administrators group

If not UAC Enabled or the user is not a member of the administrators group
installer behaves the same as it does now for a "Just me" install.

If both UAC Enabled and the user is a member of the administrators group display the UAC prompt.

* Note: this is where I need some guidance:
If the user clicks Cancel in the UAC

1. We could display a page to select an "Everyone" or "Just me" install.
2. We could continue with a "Just me" install.
3. We could display an error message and cancel the install.

I personally prefer 1

* Note: we could add a command line switch to force a "Just me" or "Everyone" installation. We should require that the process launching the installer for this scenario already has the appropriate privileges for the installation type it is trying to perform.
Instead of using two radio buttons Vista's DUN Wizard page uses a single checkbox with the image denoting that UAC will be required if this is checked. If we go with a page for the cancel case I'd suggest something along these lines. We should be able to add this when it is required to the bottom of the existing Setup Type wizard page also shown in the screen shot.
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: --- → Firefox 3 M9
To fix this right I am going to also fix the bugs I added to this bug's block list... they are just too interwoven to do individually or are simple to do at the same time.
Attached patch patch rev1 (obsolete) — Splinter Review
Attachment #282523 - Flags: review?(sspitzer)
Comment on attachment 282523 [details] [diff] [review]
patch rev1

Seth, this requires the updted 7-zip stub ( attachment #282511 [details] ) in other-licenses/7zstub/firefox/ and the UAC plugin ( attachment #282512 [details] ) in toolkit/mozapps/installer/windows/nsis/.
Attached patch patch rev2 (obsolete) — Splinter Review
The logic I am using is that if you can elevate you must elevate to use the installer. This keeps the process flow simple and the user experience the same as what one would expect after seeing the UAC dialog.

I am working on a NSIS plugin to allow setting an app as default so the call to SetDefaultBrowserVista in nsWindowsShellService.cpp will no longer be necessary. As it stands now there is the possibility of an edgecase where the required registry keys to set the app as default on Vista won't be present when SetDefaultBrowserVista is called.

This also makes the side by side install experience about as good as possible since if the user has permissions to update the StartMenuInternet keys.

I'm thinking about adding a messsagebox elevation is possible to inform the user that they will be prompted and why it is needed (e.g. updating the startmenuinternet keys).
Attachment #282523 - Attachment is obsolete: true
Attachment #282524 - Flags: review?(sspitzer)
Attachment #282523 - Flags: review?(sspitzer)
btw: we could remove nsSetDefaultBrowser.js after this lands if desired. Removing command line args is a good thing in my book but it isn't necessary to fix this bug.
There is now a NSIS plugin for setting an app as the default handler for all of its registered handlers (bug 397884).

Attached patch patch rev3 (obsolete) — Splinter Review
Includes the SetVistaDefaultApp plugin
Attachment #282524 - Attachment is obsolete: true
Attachment #282763 - Flags: review?(sspitzer)
Attachment #282524 - Flags: review?(sspitzer)
Comment on attachment 282763 [details] [diff] [review]
patch rev3

r=sspitzer, thanks to robert for walking me through this patch in person
Attachment #282763 - Flags: review?(sspitzer) → review+
two nits:

1)

+; be one installation registerred under StartMenuInternet per application since

s/registerred/registered
 
2) 
while doing the review, robert saw that he duplicated a block of code that he might want to clean up in the SetAsDefaultAppUser macro:

+  ; It is only possible to set this installation of the application as the
+  ; StartMenuInternet handler if it was added to the HKLM StartMenuInternet
+  ; registry keys.
+  ; http://support.microsoft.com/kb/297878
+
+  SetShellVarContext all  ; Set SHCTX to all users (e.g. HKLM)
+  ${FixShellIconHandler}
+
+  ${StrFilter} "${FileMainEXE}" "+" "" "" $R9
+  ClearErrors
+  ReadRegStr $0 HKLM "Software\Clients\StartMenuInternet\$R9\DefaultIcon" ""
+  ${Unless} ${Errors}
+    ${GetPathFromString} "$0" $0
+    ${GetParent} "$0" $0
+    ${If} ${FileExists} "$0"
+      ${GetLongPath} "$0" $0
+      ${If} "$0" != "$INSTDIR"
+        ; Calls after ElevateUAC won't be made if the user can elevate. They
+        ; will be made in the new elevated process if the user allows elevation.
+        ${ElevateUAC}
+
+        ${SetStartMenuInternet}
+        WriteRegStr HKCU "Software\Clients\StartMenuInternet" "" "$R9"
+      ${EndIf}
+    ${EndIf}
+  ${Else}
+    ; Calls after ElevateUAC won't be made if the user can elevate. They
+    ; will be made in the new elevated process if the user allows elevation.
+    ${ElevateUAC}
+
+    ${SetStartMenuInternet}
+    WriteRegStr HKCU "Software\Clients\StartMenuInternet" "" "$R9"
+  ${EndUnless}
Attached patch patch to be checked in (obsolete) — Splinter Review
Carrying forward r+ - this includes changes discussed with Seth and an ifdef so this doesn't break Thunderbird.
Attachment #282763 - Attachment is obsolete: true
Attachment #282797 - Flags: review+
Comment on attachment 282760 [details]
Latest NSIS Set Vista default app plugin from http://nsis.sourceforge.net/SetVistaDefaultApp_plug-in

I've uploaded a cleaned up version to
http://nsis.sourceforge.net/SetVistaDefaultApp_plug-in

that is the one I'll check in
Attachment #282760 - Attachment description: Latest NSIS Set Vista default app plugin from http://nsis.sourceforge.net/SetVistaDefaultApp_plug-in → Latest NSIS Set Vista default app plugin from http://nsis.sourceforge.net/SetVistaDefaultApp_plug-in
Attachment #282760 - Attachment is obsolete: true
Checked in to trunk

RCS file: /cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/SetVistaDefaultApp.dll,v
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/SetVistaDefaultApp.dll;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/SetVistaDefaultApp.dll,v  <--  SetVistaDefaultApp.dll
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/UAC.dll,v
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/UAC.dll;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/UAC.dll,v  <--  UAC.dll
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.29; previous revision: 1.28
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/makensis.mk;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/makensis.mk,v  <--  makensis.mk
new revision: 1.14; previous revision: 1.13
done
Checking in mozilla/other-licenses/7zstub/firefox/7zSD.sfx;
/cvsroot/mozilla/other-licenses/7zstub/firefox/7zSD.sfx,v  <--  7zSD.sfx
new revision: 1.5; previous revision: 1.4
done
Checking in mozilla/browser/components/shell/src/nsWindowsShellService.cpp;
/cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.cpp,v  <--  nsWindowsShellService.cpp
new revision: 1.50; previous revision: 1.49
done
Checking in mozilla/browser/installer/windows/nsis/installer.nsi;
/cvsroot/mozilla/browser/installer/windows/nsis/installer.nsi,v  <--  installer.nsi
new revision: 1.36; previous revision: 1.35
done
Checking in mozilla/browser/installer/windows/nsis/shared.nsh;
/cvsroot/mozilla/browser/installer/windows/nsis/shared.nsh,v  <--  shared.nsh
new revision: 1.15; previous revision: 1.14
done
Checking in mozilla/browser/installer/windows/nsis/uninstaller.nsi;
/cvsroot/mozilla/browser/installer/windows/nsis/uninstaller.nsi,v  <--  uninstaller.nsi
new revision: 1.13; previous revision: 1.12
done
Checking in mozilla/toolkit/xre/nsAppRunner.cpp;
/cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.194; previous revision: 1.193
done
Checking in mozilla/toolkit/xre/nsIWinAppHelper.idl;
/cvsroot/mozilla/toolkit/xre/nsIWinAppHelper.idl,v  <--  nsIWinAppHelper.idl
new revision: 1.3; previous revision: 1.2
done
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: in-litmus?
rstrong: During verification, I noticed that I cannot install Minefield in the default directory as a standard user.  Is this expected?
Marcia, that is correct. If you don't have write access there is no way to write in that directory and you must choose a directory to install into
Can we get this into 2.0.0.10 so it will be ready to fix bug 402976 before beta2 of 3.0 (since this affects side-by-side installs).
Flags: blocking1.8.1.10+
I can backport the patch but we will need to upgrade NSIS on the MOZILLA_1_8_BRANCH tinderboxen from 1.17 to 2.22 which is the version used on the trunk so we can use manifests in the NSIS binaries (see bug 403078)
I'm nervous about taking such a big patch (plus toolchain changes) into 2.0.0.10 (needs to land today or tomorrow, due to bug 403331), I think this has to wait for 2.0.0.11 in January
Flags: blocking1.8.1.10+ → blocking1.8.1.11+
Should we reopen this bug? It feels odd to be tracking a hot FF2.0.0.10 discussion in a closed bug?!
IMO no, typically closed is in reference to the trunk and the fixed / verified with version refers to the branch.
https://litmus.mozilla.org/show_test.cgi?id=5008 has been added to Litmus to cover this scenario.
Flags: in-litmus? → in-litmus+
Flags: wanted1.8.1.x+
Clearly we're not going to get this on the 1.8 branch anytime soon. Hopefully a fix in Firefox 3 is good enough, not going to block on it.
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.12+
Definitely, we need Bug 403078 fixed first
rob: how quickly can you get a patch for this ready? We'd like to get this in for FF2.0.0.13 if possible. (I'm working on bug#403078)
Flags: blocking1.8.1.13?
This is a huge patch, and the branch probably needs a different huge patch. If this can't land next week (before March) I don't see how we could safely take it in 2.0.0.13.

Is a reviewed branch patch by next week realistic?
Flags: wanted1.8.1.x+
Looks like this will not realistically make 2.0.0.13, maybe next time.
Flags: blocking1.8.1.13? → blocking1.8.1.13-
There's two items of work here:

1) Rob's patch - dont know enough to comment on that.

2) the nsis update - I've already started on that last night. Do you want me to stop and undo? Or can we at least get this in? I estimate being finished this afternoon.
(In reply to comment #33)
> There's two items of work here:
> 
> 1) Rob's patch - dont know enough to comment on that.
> 
> 2) the nsis update - I've already started on that last night. Do you want me to
> stop and undo? Or can we at least get this in? I estimate being finished this
> afternoon.
(Urg - hit "commit" too quickly, sorry). My nsis work is being tracked in Bug#403078, which still has "blocking1.8.1.13+", so I'll guess I continue, unless I hear otherwise?
Having the Bugzilla assign to me bookmarklet right next to a search query on the toolbar probably isn't the best idea.  Sorry for the spam.
Assignee: me → robert.bugzilla
You need to log in before you can comment on or make changes to this bug.