Closed Bug 1278248 Opened 8 years ago Closed 8 years ago

[Elevated Update] Software Update new Downloading window

Categories

(Toolkit :: Application Update, defect)

Unspecified
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla50
Tracking Status
firefox49 + verified
firefox50 --- verified

People

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

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

Attached image downloading1.png
[OS Affected]:
Mac OSX 10.9/10.10/10.11

[Regression Range]:
Good: http://archive.mozilla.org/pub/firefox/nightly/2016/05/2016-05-31-03-02-58-mozilla-central/
Bad: http://archive.mozilla.org/pub/firefox/nightly/2016/06/2016-06-01-03-02-19-mozilla-central/


[Description]:
in the case where you try to cancel an elevate update you are prompted with a  window "Update Failed" which has an OK button which upon pressing will redirect you to another window "Downloading Nightly". This "Downloading Nightly" is not what we are expecting there.


[Steps to Reproduce]:
1. Set 1 admin and 1 standard user account.
2. Login on admin account and install Nightly Firefox. (do not open FF after installation - or have a profile with automatic update set to manual)
3. Switch to standard user account.
4. Open Nightly, go to about Nightly/Update to 49.01.
5. Once the update is downloaded, restart to update.
6. Restart again to update (elevated update is triggered).
7. The admin credentials form will pop-up: press Cancel

[Actual Result]:
The "Update Failed" window will be displayed (see attachment); pressing will display a new window: "Downloading Nightly" which will show a download in progress which  will remain blocked to the "Connecting to server" status or downloads an update (4 out of 10 downloads successful). After the Download is completed, the restart window is shown again and if you restart, you will get the OSX admin authentication window (the update is still in the Elevated State) and upon cancel the outcome this time will be the Expected Result one.

[Expected Result]:
Nightly is restarted and after restart the "Update Failed" window is displayed and upon pressing Ok, the "Update Failed" window will be closed showing a message that instructs the user to manually download the build from Mozilla.org.


[Additional Notes]:
1. Also, with the "Downloading Nightly" the "partial update" syntax is introduced: the update downloaded by going to About Nightly is full update, which the update that opens in the Update Failed window is a partial one.
2. The 06.01 build is a bit more broken than the build from 06.02, the update failed window being shown when you open Nightly in Step4 (06.01) and not when you press Cancel(06.02).
Blocks: 394984
Severity: normal → major
Might this be the culprit: Bug 1271761?
Flags: needinfo?(robert.strong.bugs)
Severity: major → critical
Tracking for 49. This may block release since it affects users being able to update. Is this worse than the situation before bug 394984 was fixed in 49?  Or just a cleanup detail affecting only the multiuser system users who were affected already?
Robert, could you take this? Thanks!
Assignee: nobody → robert.strong.bugs
(In reply to Adrian Florinescu [:AdrianSV] from comment #1)
> Might this be the culprit: Bug 1271761?
Extremely doubtful

Adrian, could you set app.update.log to true and copy / paste into this bug the lines in the error console that start with *** AUS ? Thanks!
Flags: needinfo?(robert.strong.bugs) → needinfo?(adrian.florinescu)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #4)
> (In reply to Adrian Florinescu [:AdrianSV] from comment #1)
> > Might this be the culprit: Bug 1271761?
> Extremely doubtful
> 
> Adrian, could you set app.update.log to true and copy / paste into this bug
> the lines in the error console that start with *** AUS ? Thanks!
or browser console
As requested the Browser Console errors that contain 'AUS' http://pastebin.com/QNFwRLBH
The pastebin is recorded after step7 from the STR.
Flags: needinfo?(adrian.florinescu)
Thanks Adrian, I believe that I can fix this with that info!
Robert, did we land anything to address this regression yet (in a different bug maybe)?
Flags: needinfo?(robert.strong.bugs)
No, I was on vacation and will be looking at this after I catch up on a couple of reviews and such.
Flags: needinfo?(robert.strong.bugs)
Attached patch Patch (obsolete) — Splinter Review
Partial updates displayed the "errorpatching" wizard page instead of the "errors" page. Setting the |patchingFailed| property of the update object to "complete" ensures that we'll always display the desired "errors" page.
Assignee: robert.strong.bugs → spohl.mozilla.bugs
Status: NEW → ASSIGNED
Attachment #8770643 - Flags: review?(robert.strong.bugs)
Comment on attachment 8770643 [details] [diff] [review]
Patch

This should be possible without faking the update type. I have been working on this. If you want to take it let me know.
Attachment #8770643 - Flags: review?(robert.strong.bugs) → review-
Attached patch patch rev1 (obsolete) — Splinter Review
I think your approach is safer than the one I had been taking but I went ahead and made the failure explicit since the property being checked by the update ui is a custom property vs. update type.

I am concerned that the following
https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1281

will make it so update is null here
https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1293

I'm investigating and will file a new bug if that is the case. Do you know if that was manually tested? I'm going to try to create a mac specific mochitest-chrome test for the case where it has and hasn't reached DEFAULT_CANCELATIONS_OSX_MAX.
Assignee: spohl.mozilla.bugs → robert.strong.bugs
Attachment #8770643 - Attachment is obsolete: true
Attachment #8770726 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 8770726 [details] [diff] [review]
patch rev1

Review of attachment 8770726 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/update/content/updates.js
@@ +412,5 @@
> +              // page, triggered by the |STATE_DOWNLOAD_FAILED| state. This also
> +              // handles the case when an elevation was cancelled on Mac OS X.
> +              state = STATE_DOWNLOAD_FAILED;
> +            }
> +            else {

nit: move |else| to previous line
Attachment #8770726 - Flags: review?(spohl.mozilla.bugs) → review+
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #12)
> I am concerned that the following
> https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> nsUpdateService.js#1281
> 
> will make it so update is null here
> https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> nsUpdateService.js#1293
> 
> I'm investigating and will file a new bug if that is the case. Do you know
> if that was manually tested? I'm going to try to create a mac specific
> mochitest-chrome test for the case where it has and hasn't reached
> DEFAULT_CANCELATIONS_OSX_MAX.

Discussed via IRC.
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6df6c214b402
On Mac, don't download complete mar file when cancelling elevation of a partial mar file on Mac. r=spohl
Attached patch patch as landedSplinter Review
Attachment #8770726 - Attachment is obsolete: true
Attachment #8771004 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/6df6c214b402
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Comment on attachment 8771004 [details] [diff] [review]
patch as landed

Approval Request Comment
[Feature/regressing bug #]: bug 394984
[User impact if declined]: Incorrect UI is displayed to users who cancel an elevated update on OSX. QA is also blocking on this bug to sign off on bug 394984 for general release to our users. This is the last blocking bug before bug 394984 can be signed off.
[Describe test coverage new/current, TreeHerder]: manually verified
[Risks and why]: Minimal, since this is a very small and well understood patch.
[String/UUID change made/needed]: none
Attachment #8771004 - Flags: approval-mozilla-aurora?
Adrian, could you verify that the fix works for you? Thanks!
Flags: needinfo?(adrian.florinescu)
Sure thing Robert, I'll pass this along to Ovidiu to verify it.
Flags: needinfo?(adrian.florinescu) → needinfo?(ovidiu.boca)
I have tested this issue on Mac OS X 10.10 with FF 50.0a1 (2016-07-18) and I can't reproduce it. Everything works as expected, Nightly is restarted and after restart the "Update Failed" window is displayed and upon pressing Ok, the "Update Failed" window is closed showing a message that instructs the user to manually download the build from Mozilla.org.
Flags: needinfo?(ovidiu.boca)
Comment on attachment 8771004 [details] [diff] [review]
patch as landed

Needed for 49, taking it.
Attachment #8771004 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Also verified this is fixed while updating DevEdition from 2016-07-26 to latest across all Mac versions (10.9, 10.10 and 10.11).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.