Download progress icon is visible on next restart if the download failed (canceled by closing the browser)
Categories
(Firefox :: Downloads Panel, defect, P3)
Tracking
()
People
(Reporter: csasca, Assigned: mak)
References
(Depends on 1 open bug)
Details
Attachments
(3 files)
Found in
- Firefox 107.0b3
Affected versions
- Firefox 107.0b3
- Firefox 108.0a1
Tested platforms
- Affected platforms: Windows 8.1
- Unaffected platforms: Win 7 & 10, macOS, Ubuntu
Steps to reproduce
- Launch Firefox
- Access Thinkbroadband page and download the 1GB file
- While the download is in progress close Firefox
- Open Firefox with the same profile
Expected result
- The download is failed and no progress is shown on the downloads icon
Actual result
- The downloads icon keeps the progress chart even if the download is failed
Regression range
- WIll see for a regression
Additional notes
- The issue can be seen in the attachment
- Only Windows 8.1 seems to be affected
Reporter | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 1•2 years ago
|
||
I suspect the problem may actually be in Widget, since the taskbar widget is managed there and it's likely 8.1 may have some different behavior from other versions.
That said, finding someone on that version is not trivial. I don't have any machine to try atm.
Updated•2 years ago
|
Comment 2•2 years ago
|
||
I was able to reproduce this issue on a Windows 10 machine as well and I tried to get a "manual" regression range for this issue because each build needs a restart.
This is the best we could get, hopefully this helps one of our devs identify what is causing this bug:
100.0a1 (2022-04-01) - last good - https://hg.mozilla.org/mozilla-central/rev/a9b419c8a9c8f73b7ae8f18ff13a92499b3a2fa4
100.0a1 (2022-04-02) - First Bad - https://hg.mozilla.org/mozilla-central/rev/d14c155fb35a9ae59561ca33344ecb91e42dc1b1
Comment 3•2 years ago
|
||
I was able to get a better range for this issue :
hopefully it narrows it down a bit.
Comment 4•1 year ago
|
||
I was able to reproduce this issue also on Ubuntu 20.04 if after reopening Firefox the first time I retry the download and reopen Firefox again - reproduced using the latest Nightly 110.0a1 and the Firefox 102.7.0 esr.
Comment 5•1 year ago
|
||
This bug is... quite strange. Is this really Windows-specific? I don't quite see why it would be. I could definitely also repro on Win10. I guess per comment 4, it isn't actually Windows-specific.
I think the confusing thing here is that the internal state of the downloads view code thinks there's progress. The root issue is probably that that's being reported even though there are no actual in-progress downloads. But I don't see why that'd be different on macOS or Linux. Anyway, I'm concerned that that would also break other things. So clearing some flags for re-triage.
Maybe more importantly: does this also happen for other downloads that aren't first blocked by the mixed content blocker?
(the regression window has no clear culprits - I expect it's a race condition of sorts, maybe?)
Reporter | ||
Comment 6•1 year ago
|
||
Yep, I was able indeed to reproduce it on Windows 10 with 111.0b1 as well but after some several tries, not as consistently as on Windows 8.1. Seems that this is happening on other normal downloads which are not blocked as well (see attachment).
Assignee | ||
Comment 7•1 year ago
|
||
I don't know if it's the same thing (there may be a relation) but I can reproduce something close with simple steps:
- click on a download link on thinkbroadband
- when the downloads panel shows the download is insecure click on allow
- pause the download, it will change straight to Failed, but the download icon is still showing progress
Even if it's not the same bug, maybe it's a good pointer to find the problem.
Assignee | ||
Comment 9•1 year ago
|
||
On startup _updateView is just setting percentComplete in the view (indicator in this case), the value is taken from a summary built by summarizeDownloads() that for non succeeded downloads just looks for download.hasProgress
That's an interesting choice, because it considers progress regardless of download failed state, it looks like it comes from a time where failing would remove the .part file in any case. Today that doesn't seem to be the case.
If I retry the failed download, it will actually resume from the progress, so the .part file is correctly retained.
So, there is an inconsistency between the Failed status we show in the ui and the progress indicator.
A very trivial fix would be to change the above condition from else if (download.hasProgress)
to else if (download.hasProgress && !download.error)
, that would at least synchronize the status between the list and the indicator, a Failed download doesn't indeed show a progress bar.
Regarding my STR, I'm still not sure why Pause is Failing the download, that sounds like some bug related to blocked downloads management.
Assignee | ||
Comment 10•1 year ago
|
||
Assigning to myself to keep track of this, but if anyone wants to steal just let me know. The issue and solution are in comment 9, it just needs some kind of test.
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 11•1 year ago
|
||
The problem may be a little bit more complex than I outlined. It's not actually about skipping downloads with a download.error, but it's because canceling a download may set download.error.
Unblock() is actually doing download.start().catch(ex => this.error = err)
that is copying the "Canceled download" error into the download object.
That object may then be serialized and deserialized bringing the error along with it.
Our code always checks download.error before download.canceled, to determine the state of a download.
This is the reason for which my unblock then pause fails and it ends up generating a download that is .canceled, .hasPartialData, but also has an .error.
Catalin, do you remember if the cases where you noticed the failure were all for downloads you "Unblocked" in the ui (maybe because http mixed content).
Assignee | ||
Comment 12•1 year ago
|
||
Another case I found, pausing a download and closing the browser, aborts the download and on the next startup it should be shown as paused, instead it has an "abort" error.
Assignee | ||
Comment 13•1 year ago
|
||
Reporter | ||
Comment 14•1 year ago
|
||
Hey Marco, it happens on normal downloads as well, tried that and mentioned in Comment 6, so all downloads should be affected by this, not only the blocked/unblocked ones.
Assignee | ||
Comment 15•1 year ago
|
||
My patch should fix the issue, with it I can consistently pause/cancel/retry and restart the browser where ongoing downloads automatically continue from where they were. Though I must write some test, even if a shutdown test will likely be out of scope here and require a follow-up bug for Marionette.
Just a small summary of my findings:
Originally we used to restart downloads automatically at th next session when the user closed the browser with ongoing downloads.
That broke because:
- there's "network going offline" detection code, that was intended to handle the "work offline" option, but network sometimes also fires it on shutdown. It could make sense, but that cancels downloads, and then on the next startup we see they were Canceled and don't start them.
- The above notification is intermittent on shutdown, sometimes it happens, sometimes not, when it happens downloads are canceled, when it doesn't happen downloads are aborted. The NS_ERROR_ABORT is then copied in .error and serialized to downloads.json. On the next startup we'll see them as Failed.
- Additionally, the Unblock() code path is copying over any exception to .error, also the exception generated by a call to cancel(). download.error is already set by start(), there should be no reason to further set it.
- a download may have both .error and .errorObj, on retry() we clear the former, but not the latter, so a download that stores an errorObj, is restarted and then serialized again, will bring the original errorObj with itself and continue being marked as failed even if it was restarted succesfully
Updated•1 year ago
|
Updated•1 year ago
|
Comment 16•1 year ago
|
||
Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/1277131e3259 Canceled downloads are shown as failed. r=Gijs
Comment 17•1 year ago
|
||
bugherder |
Updated•1 year ago
|
Comment 18•1 year ago
|
||
Verified as fixed in our latest Nightly build 113.0a1 (2023-03-15).
Comment 19•1 year ago
|
||
The patch landed in nightly and beta is affected.
:mak, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox112
towontfix
.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 20•1 year ago
|
||
This is broken from quite a while, and I think it will benefit from Nightly testing.
So I'd just let it ride the train to release 113.
Description
•