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

Status: RESOLVED FIXED

(Firefox :: Installer, defect)

Description

[Robert Strong (they/them - no direct email)]

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.

Comment 1

[Robert Strong (they/them - no direct email)]

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.

Comment 2

[Robert Strong (they/them - no direct email)]

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.

Comment 3

[Robert Strong (they/them - no direct email)]

Attachment: Everyone vs. Just me DUN Wizard page

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.

Comment 4

[Robert Strong (they/them - no direct email)]

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.

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: New 7-zip stub

Comment 6

[Robert Strong (they/them - no direct email)]

Attachment: Latest NSIS UAC plugin from http://nsis.sourceforge.net/UAC_plug-in

Comment 7

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Comment 8

[Robert Strong (they/them - no direct email)]

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/.

Comment 9

[Robert Strong (they/them - no direct email)]

Attachment: patch rev2

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).

Comment 10

[Robert Strong (they/them - no direct email)]

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.

Comment 11

[Robert Strong (they/them - no direct email)]

There is now a NSIS plugin for setting an app as the default handler for all of its registered handlers (bug 397884).

Comment 12

[Robert Strong (they/them - no direct email)]

Attachment: Latest NSIS Set Vista default app plugin from http://nsis.sourceforge.net/SetVistaDefaultApp_plug-in

Comment 13

[Robert Strong (they/them - no direct email)]

Attachment: patch rev3

Includes the SetVistaDefaultApp plugin

Comment 14

[(not reading, please use [e-mail] instead)]

Comment on attachment 282763 [details] [diff] [review]
patch rev3

r=sspitzer, thanks to robert for walking me through this patch in person

Comment 15

[(not reading, please use [e-mail] instead)]

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}

Comment 16

[Robert Strong (they/them - no direct email)]

Attachment: patch to be checked in

Carrying forward r+ - this includes changes discussed with Seth and an ifdef so this doesn't break Thunderbird.

Comment 17

[Robert Strong (they/them - no direct email)]

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

Comment 18

[Robert Strong (they/them - no direct email)]

Attachment: patch checked in (fixed ExecCodeSegment and a couple of comments)

Comment 19

[Robert Strong (they/them - no direct email)]

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

Comment 20

[Marcia Knous [:marcia]]

rstrong: During verification, I noticed that I cannot install Minefield in the default directory as a standard user.  Is this expected?

Comment 21

[Robert Strong (they/them - no direct email)]

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

Comment 22

[Mike Schroepfer]

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).

Comment 23

[Robert Strong (they/them - no direct email)]

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)

Comment 24

[Daniel Veditz [:dveditz]]

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

Comment 25

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Should we reopen this bug? It feels odd to be tracking a hot FF2.0.0.10 discussion in a closed bug?!

Comment 26

[Robert Strong (they/them - no direct email)]

IMO no, typically closed is in reference to the trunk and the fixed / verified with version refers to the branch.

Comment 27

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=5008 has been added to Litmus to cover this scenario.

Comment 28

[Daniel Veditz [:dveditz]]

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.

Comment 29

[Robert Strong (they/them - no direct email)]

Definitely, we need Bug 403078 fixed first

Comment 30

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

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)

Comment 31

[Daniel Veditz [:dveditz]]

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?

Comment 32

[Daniel Veditz [:dveditz]]

Looks like this will not realistically make 2.0.0.13, maybe next time.

Comment 33

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

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.

Comment 34

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(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?

Comment 35

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.