922227 - UAC dialog on all nightly updates for the last few days

Status: RESOLVED WONTFIX

(Toolkit :: Application Update, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: logs.zip

I'm running Windows 7 and I'm seeing a UAC dialog for every nightly update as of about five days ago. Looks like a cert problem, see attached logs.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: MaintenanceService.txt

Here's a dump of HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\MaintenanceService too.

Comment 2

[Brian R. Bondy [:bbondy]]

First appeared on 09-27 which is consistent with when bug 803531 landed.  Also error indicates cert mismatch.  Most likely cause is that registry is not updated.  The logs show that the post update process is being executed though.

Comment 3

[Brian R. Bondy [:bbondy]]

Talked to bent on irc.

What's happening is that the PostUpdate bails out early when uninstall.log doesn't exist, so the registry entries for the cert check aren't getting updated:
http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/windows/nsis/common.nsh#l5310

[ 14:15:12 ] [ bbondy ] does this file exist? C:\Program Files (x86)\Nightly\uninstall\uninstall.log
[ 14:15:17 ] [ bbondy ] I'm guessing no
[ 14:15:28 ] [ bbondy ] because when it doesn't we don't run postupdate, and that's what updates the registry where the cert is stored
[ 14:15:59 ] [ bent ] no, no uninstall.log


rstrong what's your suggestion here? Should we finally always execute PostUpdate even for zip builds? Or should we do nothing? or?

Similar issues have come up a few times in the past, we decided not to do anything so that zip build users won't have their registry affected as much.

I'd prefer we just get rid of that check though to avoid wasting time on things like this in the future.  We can say zipped builds aren't supported for issues like this, but they waste time as bugs like this come up.  I'd prefer to say "not modifying the registry is not supported".

Comment 4

[Brian R. Bondy [:bbondy]]

An alternate fix would be to break out PostUpdate into 2 functions. One always gets called and would include things like the cert registry info.  The other would be the less important stuff.  I'd prefer to just always run everything though for the sake of future maintenance.

Comment 5

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

(In reply to Brian R. Bondy [:bbondy] from comment #3)
> Talked to bent on irc.
> 
> What's happening is that the PostUpdate bails out early when uninstall.log
> doesn't exist, so the registry entries for the cert check aren't getting
> updated:
> http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/
> windows/nsis/common.nsh#l5310
> 
> [ 14:15:12 ] [ bbondy ] does this file exist? C:\Program Files
> (x86)\Nightly\uninstall\uninstall.log
> [ 14:15:17 ] [ bbondy ] I'm guessing no
> [ 14:15:28 ] [ bbondy ] because when it doesn't we don't run postupdate, and
> that's what updates the registry where the cert is stored
> [ 14:15:59 ] [ bent ] no, no uninstall.log
> 
> 
> rstrong what's your suggestion here? Should we finally always execute
> PostUpdate even for zip builds? Or should we do nothing? or?
We've had pushback from users about updating the registry for people using zip builds and there aren't many people using zip builds for daily browsing vs. checking for regressions. The main reason I dislike it is if we update registry key it isn't clear that there is a mechanism to clean up the registry entries. 

I'd prefer going with bug 922241.

Comment 6

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

Note: portable Firefox removes uninstall.log to prevent it from adding registry keys as well.

Comment 7

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

I'd also prefer not supporting zip builds and installer builds. Also, Windows zip builds are already considered unsupported.

Comment 8

[Brian R. Bondy [:bbondy]]

The clean way to clear the registry would be to use an installer to the same install location and then uninstall it. If someone really wants to clean it, and it's not a supported environment, I'd favor making them do a little extra work to cleanup rather than making them have various bugs. And worse, wasting developer time investigating problems.

Two example bugs from the past with the exact same issue:
https://bugzilla.mozilla.org/show_bug.cgi?id=806977#c10
https://bugzilla.mozilla.org/show_bug.cgi?id=780961#c13

I agree we should do bug 922241 but I also think we should do what I proposed in Comment 3.  uninstall.log may not exist even after bug 922241 for 2 classes of people 1) Those who manually unzip the install, and 2) existing users.

If you still disagree did you want this marked as invalid or did you want the alternate fix mentioned in comment 4?  At least I'm getting faster at spotting issues like this in any case, but other people may spend time here in the future as well.

I should also mention that firefox itself modifies the registry so if you don't use an uninstaller you aren't free from having a clean removal.

Comment 9

[Brian R. Bondy [:bbondy]]

This exact issue also came up recently for zipped builds not getting a default tile created on the win8 start screen by the way.  Pretty sure I've ran into it at least 1 or 2 other times than that and the above 2 quoted bugs too.

Comment 10

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

I've run into many similar bugs in the past and don't like it either though it happens significantly less often with the current implementation even with the bugs you mentioned than it use to... I'm just trying to come up with a reasonable compromise between the different use cases.

BTW: The concern expressed about post update updating the registry was about it adding keys for zip builds and not about cleaning it after uninstall so having a workaround for cleaning doesn't address the concern.

So, with bug 922241 fixed I am less concerned about people not liking that updating will modify the registry. I am still concerned about portable Firefox though since a usb stick can end up in many different machines.

I think it is reasonable to remove the check with bug 922241 fixed but a new method for portable Firefox to prevent updating the registry should be added and communicated to the maintainers of portable Firefox.

Comment 11

[Brian R. Bondy [:bbondy]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #10)
> BTW: The concern expressed about post update updating the registry was about
> it adding keys for zip builds and not about cleaning it after uninstall so
> having a workaround for cleaning doesn't address the concern.

Right but keys will be added at other places too, like if you set the default.

> So, with bug 922241 fixed I am less concerned about people not liking that
> updating will modify the registry. I am still concerned about portable
> Firefox though since a usb stick can end up in many different machines.
>
> I think it is reasonable to remove the check with bug 922241 fixed but a new
> method for portable Firefox to prevent updating the registry should be added
> and communicated to the maintainers of portable Firefox.

I sent the maintainer an email to see if they purposely don't include the file or not. I'll follow up back here if I get a response, and with a patch once bug 922241 is implemented. Thanks!

Comment 12

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

(In reply to Brian R. Bondy [:bbondy] from comment #11)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #10)
> > BTW: The concern expressed about post update updating the registry was about
> > it adding keys for zip builds and not about cleaning it after uninstall so
> > having a workaround for cleaning doesn't address the concern.
> 
> Right but keys will be added at other places too, like if you set the
> default.
Right and the compromise here is that the user has to initiate this action which updates the registry vs. the updater doing this behind the scene without it being user initiated at least in some cases.

> > So, with bug 922241 fixed I am less concerned about people not liking that
> > updating will modify the registry. I am still concerned about portable
> > Firefox though since a usb stick can end up in many different machines.
> >
> > I think it is reasonable to remove the check with bug 922241 fixed but a new
> > method for portable Firefox to prevent updating the registry should be added
> > and communicated to the maintainers of portable Firefox.
> 
> I sent the maintainer an email to see if they purposely don't include the
> file or not. I'll follow up back here if I get a response, and with a patch
> once bug 922241 is implemented. Thanks!
They *might* no longer remove it but I can say that they used this method in the past to prevent updating from adding registry keys. iirc they also had special scripts for launching that handled the default browser case.

Comment 13

[Mark Hammond [:markh] [:mhammond]]

I haven't seen this for a while - close as WFM?

Comment 14

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

We're not going to support this for zip builds and there is a simple workaround by using the installer.