Closed Bug 1171792 Opened 7 years ago Closed 7 years ago

An update is attempted from an update notification when it is not possible for the logged in user to update

Categories

(Toolkit :: Application Update, defect)

38 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox38 --- wontfix
firefox39 + verified
firefox38.0.5 --- wontfix
firefox40 + verified
firefox41 + verified
firefox-esr38 39+ verified

People

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

References

Details

Attachments

(6 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150513174244

Steps to reproduce:

Used automated update pop-up. Mac OS v10.10.3


Actual results:

It appeared to download then requested Restart and I selected Restart Firefox.


Expected results:

New version of Firefox should have installed and started.  I had to manually start Firefox and it was the old version.
Component: Untriaged → Application Update
OS: Unspecified → Mac OS X
Product: Firefox → Toolkit
Hardware: Unspecified → x86
First, please don't reinstall so we can troubleshoot exactly what is going on.

In the Mac finder and in your home directory please open Library/Caches/Mozilla/updates/Applications/Firefox/updates

In that directory there should be files named last-update.log and backup-update.log. Please attach them to this bug using the "Add an attachment" link.

Then look in the 0 directory. If there is a file named update.log please attach it to this bug as well. Thanks!
Flags: needinfo?(markgradisar)
Requested files named last-update.log and backup-update.log in a zip file
Flags: needinfo?(markgradisar)
Whiteboard: See attached requested file.
There was an error renaming Contents/_CodeSignature/CodeResources

Could you launch a terminal and cd into /Applications/Firefox.app/Contents/_CodeSignature/
Then type ls -l and copy the output and paste it into a comment in this bug.

For comparison purposes please cd into /Applications/Firefox.app/Contents/Resources/
Then type ls -l application.ini and paste it into a comment in this bug.

Thanks!
Flags: needinfo?(markgradisar)
Attached file Archive.zip
Resources & Signature ls
Flags: needinfo?(markgradisar)
Attached file last-update.log
Attachment #8616305 - Attachment is obsolete: true
Thanks! I think I see what is going on and will have a Mac available tomorrow to verify if my suspicion is correct. Please don't re-install until after I verify it since I might need you to check something else.
Hi Mark, I think I now have an understanding of the cases where this happens. Can you tell me what the about window displays under the Firefox menu? Thanks
Flags: needinfo?(markgradisar)
See attached 'about' window
Flags: needinfo?(markgradisar)
Thanks Mark. This bug has two parts. First, there is a permissions check that is missing in the update window code. Second, bug 394984 prevents people that don't have permissions from updating. I'll use this bug to address the first part. The second part has been difficult to fix due to a lack of resources. I've talked this over with a couple of people and I will hopefully be able to work on that bug later this month.

You can resolve this issue on your system by removing the existing install and reinstalling. Keep in mind (per bug 394984) that the person that performs the install will be the only person that can update until bug 394984 is fixed.
Assignee: nobody → robert.strong.bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
[Tracking Requested - why for this release]:
This prevents notifying the user that an update is available when they are not able to update.
OS: Mac OS X → All
Hardware: x86 → All
Summary: I am not able to update to 38.0.5 from 38.0.1 → An update is attempted when it is not possible to update
Summary: An update is attempted when it is not possible to update → An update is attempted from an update notification when it is not possible to update
Whiteboard: See attached requested file.
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #12)
> Additional try push
> https://bugzilla.mozilla.org/show_bug.cgi?id=1171792
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c91e88a55c5b
Attachment #8616505 - Flags: review?(spohl.mozilla.bugs) → review+
Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/51056edec2f3
Flags: in-testsuite+
Target Milestone: --- → mozilla41
Comment on attachment 8616505 [details] [diff] [review]
patch rev1

Approval Request Comment
[Feature/regressing bug #]: I've looked but wasn't able to find the regressing bug.
[User impact if declined]: Users that don't have permission to update will attempt to update without ever displaying the manual upgrade page informing them that they should update.
[Describe test coverage new/current, TreeHerder]: Tests in tree
[Risks and why]: Minimal. The client changes are to check if the user can apply updates when there is an update available and there is a corresponding check in the same file for when checking for updates for the same case that has worked for quite some time. There are also tests for both cases in the patch.
[String/UUID change made/needed]: None
Attachment #8616505 - Flags: approval-mozilla-esr38?
Attachment #8616505 - Flags: approval-mozilla-beta?
Attachment #8616505 - Flags: approval-mozilla-aurora?
[Tracking Requested - why for this release]: tracked for 39, 40, 41, and esr38+, don't have privileges to esr31, will uplift when solution is found.
Henrik, I wonder if you're the right person to ask. This sounds like a mac-specific regression and it would be good to have coverage for this in the update tests. Who would be the right person to ask to write those tests?
Flags: needinfo?(hskupin)
Robert, in regards an automated Marionette update test, would it be important to utilize the notification popup here? If that is the case a test would not be possible given that this is a system notification and we don't have access to it. I|m also not sure about the permissions. We run as the same user who installed the build. So we always have the permission to update.
Flags: needinfo?(hskupin)
(In reply to Liz Henry (:lizzard) from comment #17)
> Henrik, I wonder if you're the right person to ask. This sounds like a
> mac-specific regression and it would be good to have coverage for this in
> the update tests. Who would be the right person to ask to write those tests?
Liz, this is not a Mac specific regression.

(In reply to Henrik Skupin (:whimboo) from comment #18)
> Robert, in regards an automated Marionette update test, would it be
> important to utilize the notification popup here? If that is the case a test
> would not be possible given that this is a system notification and we don't
> have access to it. I|m also not sure about the permissions. We run as the
> same user who installed the build. So we always have the permission to
> update.
Henrik, there is already a test in tree and a marionette test isn't needed.
Robert, my mistake on it being a Mac specific problem.  

On tests: we didn't appear to catch this in our current update testing. It sounds like we don't know when this regression happened or which versions it affects.  So it seems logical to ask what new update tests we could add to cover this case. 

Can you help us verify the fix, or describe how to reproduce the problem so QE can try to verify it? As you can imagine it is somewhat unnerving to uplift this with a week and a half to go for beta 39.
Flags: needinfo?(robert.strong.bugs)
(In reply to Liz Henry (:lizzard) from comment #20)
> Robert, my mistake on it being a Mac specific problem.  
> 
> On tests: we didn't appear to catch this in our current update testing. It
> sounds like we don't know when this regression happened or which versions it
> affects.  So it seems logical to ask what new update tests we could add to
> cover this case. 
Tests are being added in this bug which is why tests didn't catch this bug and hence the in-testsuite+ and the release driver approval request has "Describe test coverage new/current, TreeHerder]: Tests in tree". When I wrote the mochitest chrome tests years ago it was not possible to have a test for this and new code has made it possible.

> Can you help us verify the fix, or describe how to reproduce the problem so
> QE can try to verify it? As you can imagine it is somewhat unnerving to
> uplift this with a week and a half to go for beta 39.
No problem and I always do after the uplift request is approved since the time is better spent elsewhere if the uplift request is not approved.
Flags: needinfo?(robert.strong.bugs) → needinfo?(lhenry)
Comment on attachment 8616505 [details] [diff] [review]
patch rev1

Approved for uplift to aurora and beta. rstrong will verify on both 39 and 40 and has added test coverage.
Flags: needinfo?(lhenry)
Attachment #8616505 - Flags: approval-mozilla-beta?
Attachment #8616505 - Flags: approval-mozilla-beta+
Attachment #8616505 - Flags: approval-mozilla-aurora?
Attachment #8616505 - Flags: approval-mozilla-aurora+
Sheriffs, I'll land this on the branches so I can first verify the patch applies cleanly and the tests pass.
Patch applies cleanly and tests pass on aurora. Patch didn't apply cleanly to beta and esr38 as I expected and I already have patches for them. I'll submit them after I verify the tests pass.
Attached patch beta patchSplinter Review
All tests pass

BTW: I have verified that without the client changes test_0172_notify_noPerms_manual.xul fails as it should and passes with the client changes.
Attachment #8620753 - Flags: review+
Comment on attachment 8616505 [details] [diff] [review]
patch rev1

I'll submit a patch that applies cleanly to esr38 after I verify the tests pass and request esr38 on it.
Attachment #8616505 - Flags: approval-mozilla-esr38? → checkin+
Attached patch patch esr38Splinter Review
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Some users won't be notified of updates.
User impact if declined: Users that don't have permission to update will attempt to update without ever displaying the manual upgrade page informing them that they should update.
Fix Landed on Version: 39
Risk to taking this patch (and alternatives if risky): Minimal. The client changes are to check if the user can apply updates when there is an update available and there is a corresponding check in the same file for when checking for updates for the same case that has worked for quite some time. There are also tests for both cases in the patch.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8620787 - Flags: review+
Attachment #8620787 - Flags: approval-mozilla-esr38?
I built and ran the tests on esr38 as well so all should be good with the esr38 patch
Steps to reproduce:

On a Windows system install a version of Firefox that has this patch and has an update available for it in the default location. Create a Windows account that is not a member of the administrators group and is a member of the users group (typically the default when creating an account on recent version of Windows). Login as that user and launch Firefox. Immediately open tools -> options -> advanced -> update and uncheck "Use a background service to install updates". Wait until the background update check notifies you that an update is available by displaying the large update ui.

Without the patch if you try to update via the ui it should try and end up displaying the UAC prompt where it asks you to enter a username and password.

With the patch it should inform you that there is an update available and provide a link to download it.
Duplicate of this bug: 1162363
Duplicate of this bug: 1160452
Duplicate of this bug: 1065820
Comment on attachment 8620787 [details] [diff] [review]
patch esr38

Approving uplift to ESR 39 (38.1.0) as this will impact firefox update scenarios esp. the background service update option.
Attachment #8620787 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Adding a qe-verify flag so we test the fix on this one in time for ESR 38.1.0 release.
Flags: qe-verify+
Duplicate of this bug: 1176762
Duplicate of this bug: 1176765
Verified this issue as fixed using Latest Nightly 41.0a1, latest Aurora 40.0a2 and Firefox 39 beta 7.
Tested on Windows 7 64-bit and Mac OS X 10.10.4 with the STR from comment 31 and got the expected results.

Leaving this bug resolved until I'll verify it using esr38.
It seems that ESR is not quite fixed. I've tried to update from ESR 38.0 using a Guest account and it still tries to update once (the update is downloaded, browser restarted to apply update but still 38.0).

Afterwards, no notification of an update was received in more than an hour.

If the user wants to update manually, he is notified that an update is available to download. Another issue here is that the download link points to the Release instead of ESR.

Prints: http://imgur.com/a/DtNKw
(In reply to Cornel Ionce [QA] from comment #42)
> It seems that ESR is not quite fixed.
Can you provide the link to the build you are using to test and the steps you performed?
Flags: needinfo?(cornel.ionce)
I've downloaded the ESR 38.0 build from: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/38.0esr/mac/en-US/

1. Install the ESR 38.0 build from the main account.
2. Switch to a Guest account.
3. Open ESR 38.0 and wait for the update notification.
4. Restart the browser after the update was downloaded, as required.(at this point the update was not installed)
5. Go to Firefox -> About Firefox (Update available for download notification)

Note: I've tested on Mac OS X 10.10.4.
Flags: needinfo?(cornel.ionce)
The build available from
http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/38.0esr/mac/en-US/

is from May 5th so I highly doubt it has the fix.
To verify this first you will need a build that includes the patch from this bug. I installed the builds available here.

http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/38.1.0esr-candidates/build1/mac/en-US/

http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/38.1.0esr-candidates/build1/win32/en-US/

Since the latest esr builds don't have an update available a custom update will need to be used.
After launching the build installed from the above urls open about:config.
Add a new string preference with the name app.update.url.override and a value of
http://exchangecode.com/robert/work/snippets/bug1171792.xml

You should be able to verify the bug now using the steps used previously though you might need to restart. I already verified Windows works as expected.
By creating the " app.update.url.override" string and setting it to "http://exchangecode.com/robert/work/snippets/bug1171792.xml" I was able to confirm this fix for ESR 38.

Screenshot: http://i.imgur.com/1QgpoIP.jpg

Thanks Robert!
Status: RESOLVED → VERIFIED
Thank you Cornel!
Summary: An update is attempted from an update notification when it is not possible to update → An update is attempted from an update notification when it is not possible for the logged in user to update
See Also: → 394984
Duplicate of this bug: 1126957
You need to log in before you can comment on or make changes to this bug.