Download Manager regression in FF32 beta 9 update

VERIFIED FIXED in Firefox 32

Status

()

Firefox
Downloads Panel
--
major
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: philipp, Assigned: mmc)

Tracking

(Depends on: 1 bug, {regression})

32 Branch
Firefox 34
All
Windows 7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?
qe-verify +

Firefox Tracking Flags

(firefox31 unaffected, firefox32+ verified, firefox33 verified, firefox34 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8477886 [details]
download stuck

this is a major regression in the latest update from ff32 beta 8 to beta 9 and was reported in sumo threads and on irc:
https://support.mozilla.org/questions/1016710
https://support.mozilla.org/questions/1016723

STR in a clean profile:
- download & save any file, for example: http://download.documentfoundation.org/libreoffice/stable/4.3.0/win/x86/LibreOffice_4.3.0_Win_x86.msi.torrent
- download manager will be stuck on "unknown time remaining"
- when you try to clear the download this won't work, its status will change to "paused"
(Reporter)

Comment 1

4 years ago
this is apparently related to safe-browsing. when compared to beta 8, this string is missing in about:config:
 browser.safebrowsing.appRepURL;https://sb-ssl.google.com/safebrowsing/clientreport/download?key=%GOOGLE_API_KEY%

once manually added, the problem goes away...
(Reporter)

Updated

4 years ago
Component: Untriaged → Downloads Panel

Comment 2

4 years ago
It's fixed in FF33+.
(Reporter)

Updated

4 years ago
Blocks: 1055670

Comment 3

4 years ago
[Tracking Requested - why for this release]:
status-firefox31: --- → unaffected
status-firefox33: --- → unaffected
status-firefox34: --- → unaffected
tracking-firefox32: --- → ?
Keywords: regression

Updated

4 years ago
Duplicate of this bug: 1057767
Matt, are you able to reproduce? Seems strange according go https://bugzilla.mozilla.org/show_bug.cgi?id=1055670#c32
Created attachment 8477912 [details] [diff] [review]
Remove references to appRepUrl completely (
Assignee: nobody → mmc
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8477912 - Flags: review?(gpascutto)
Flags: needinfo?(mwobensmith)
status-firefox32: --- → affected
tracking-firefox32: ? → +

Updated

4 years ago
Duplicate of this bug: 1057797
So the bug is that if the pref is gone, then this:
NS_ENSURE_SUCCESS(Preferences::GetCString(PREF_SB_APP_REP_URL, &serviceUrl), NS_ERROR_NOT_AVAILABLE);
will fail to call OnComplete(false, NS_ERROR_NOT_AVAILABLE), right?

https://bugzilla.mozilla.org/show_bug.cgi?id=1055670#c32 Note the nuance "pref is blank" vs "pref does not appear" Ouch! Doesn't explain why it appeared to work, though.
(In reply to Loic from comment #2)
> It's fixed in FF33+.

That's because it's Aurora, the disabling from bug 1055670 only applies to Beta and Release branches.
Comment on attachment 8477912 [details] [diff] [review]
Remove references to appRepUrl completely (

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

I hope nothing else depends on that pref actually being there :-/
Attachment #8477912 - Flags: review?(gpascutto) → review+
(In reply to Gian-Carlo Pascutto [:gcp] from comment #9)
> So the bug is that if the pref is gone, then this:
> NS_ENSURE_SUCCESS(Preferences::GetCString(PREF_SB_APP_REP_URL, &serviceUrl),
> NS_ERROR_NOT_AVAILABLE);
> will fail to call OnComplete(false, NS_ERROR_NOT_AVAILABLE), right?

Monica, you should file a follow-up bug for this BTW.

Comment 13

4 years ago
I can confirm that adding "browser.safebrowsing.appRepURL;https://sb-ssl.google.com/safebrowsing/clientreport/download?key=%GOOGLE_API_KEY%" to about:config immediately fixes the problem.
(In reply to Gian-Carlo Pascutto [:gcp] from comment #9)
> So the bug is that if the pref is gone, then this:
> NS_ENSURE_SUCCESS(Preferences::GetCString(PREF_SB_APP_REP_URL, &serviceUrl),
> NS_ERROR_NOT_AVAILABLE);
> will fail to call OnComplete(false, NS_ERROR_NOT_AVAILABLE), right?

Yes. I think this is the safest option that we have for fixing at this point, so I'm going to go ahead and check in then nominate for uplift.

If anyone who is affected by this bug could try this build *without* modifying any preferences, I would appreciate it: https://tbpl.mozilla.org/?tree=Try&rev=6474ccc9d73c
Comment on attachment 8477912 [details] [diff] [review]
Remove references to appRepUrl completely (

Approval Request Comment
[Feature/regressing bug #]: https://bugzilla.mozilla.org/show_bug.cgi?id=1055670

[User impact if declined]: Downloads stuck on Windows

[Describe test coverage new/current, TBPL]: Please wait for someone to manually verify TBPL push on windows

[Risks and why]: High risk since it's late in cycle. Alternative is worse, though.

[String/UUID change made/needed]: none.
Attachment #8477912 - Flags: approval-mozilla-beta?
Attachment #8477912 - Flags: approval-mozilla-aurora?
(In reply to Gian-Carlo Pascutto [:gcp] from comment #12)
> (In reply to Gian-Carlo Pascutto [:gcp] from comment #9)
> > So the bug is that if the pref is gone, then this:
> > NS_ENSURE_SUCCESS(Preferences::GetCString(PREF_SB_APP_REP_URL, &serviceUrl),
> > NS_ERROR_NOT_AVAILABLE);
> > will fail to call OnComplete(false, NS_ERROR_NOT_AVAILABLE), right?
> 
> Monica, you should file a follow-up bug for this BTW.

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1057848
> If anyone who is affected by this bug could try this build *without*
> modifying any preferences, I would appreciate it:
> https://tbpl.mozilla.org/?tree=Try&rev=6474ccc9d73c

Correction: please set browser.safebrowsing.appRepURL to empty and try a windows build from

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mchew@mozilla.com-6474ccc9d73c/try-win32/
(Reporter)

Comment 19

3 years ago
thank you, when i tested your try build with a clean profile, downloads finished without a problem out of the box and when browser.safebrowsing.appRepURL is manually set to an empty string as well. i hope others can confirm that...

Comment 20

3 years ago
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #18)
> > If anyone who is affected by this bug could try this build *without*
> > modifying any preferences, I would appreciate it:
> > https://tbpl.mozilla.org/?tree=Try&rev=6474ccc9d73c
> 
> Correction: please set browser.safebrowsing.appRepURL to empty and try a
> windows build from
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mchew@mozilla.com-
> 6474ccc9d73c/try-win32/

I tried.
In both cases, browser.safebrowsing.appRepURL empty or with its default string chain, the download finishes.

Updated

3 years ago
Duplicate of this bug: 1057876

Updated

3 years ago
Duplicate of this bug: 1057851

Updated

3 years ago
Duplicate of this bug: 1057833

Updated

3 years ago
Duplicate of this bug: 1057784
Duplicate of this bug: 1057886
OS: Windows 7 → All
Hardware: x86_64 → All
The regression is Windows specific though admittedly we don't have generic "Windows" platform in Bugzilla.
Duplicate of this bug: 1058003

Updated

3 years ago
Duplicate of this bug: 1058020

Updated

3 years ago
Duplicate of this bug: 1057974

Updated

3 years ago
Duplicate of this bug: 1057895
(In reply to Gian-Carlo Pascutto [:gcp] from comment #26)
> The regression is Windows specific though admittedly we don't have generic
> "Windows" platform in Bugzilla.

Sorry, didn't wanted to change the platform but only the bits. Reverted now.
OS: All → Windows 7

Updated

3 years ago
Duplicate of this bug: 1058037
https://hg.mozilla.org/mozilla-central/rev/8be3a7df7779
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34

Updated

3 years ago
Flags: qe-verify+

Updated

3 years ago
Flags: in-testsuite?
Flags: in-qa-testsuite?(hskupin)
Comment on attachment 8477912 [details] [diff] [review]
Remove references to appRepUrl completely (

mmc confirmed that this has been tested and that there are no additional places where the pref that caused the issue is referenced. Approved for aurora and beta.
Attachment #8477912 - Flags: approval-mozilla-beta?
Attachment #8477912 - Flags: approval-mozilla-beta+
Attachment #8477912 - Flags: approval-mozilla-aurora?
Attachment #8477912 - Flags: approval-mozilla-aurora+
Thanks lmandel, I'm in meetings for the next hour so if someone wants to check in before then, please go ahead.
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #6)
> Matt, are you able to reproduce? Seems strange according go
> https://bugzilla.mozilla.org/show_bug.cgi?id=1055670#c32


Talking to Monica, my mistake was that I tested the download with EXEs that were already signed, which skip the application reputation check. I re-tested with an unsigned EXE and I can reproduce this problem. I will verify the fix when it's checked in.
Flags: needinfo?(mwobensmith)
https://hg.mozilla.org/releases/mozilla-aurora/rev/e1df4eee7044
https://hg.mozilla.org/releases/mozilla-beta/rev/92ad4cfa9435

Note: I am *not* sure if what TBPL produces is the same as the actual release build, due to things like configuring the google safebrowsing api key. Nevertheless, it would be great if Matt or Juan could test on Windows when those builds are complete.
status-firefox32: affected → fixed
status-firefox33: unaffected → fixed
status-firefox34: unaffected → fixed

Comment 38

3 years ago
(In reply to Loic from comment #2)
> It's fixed in FF33+.

Now it's getting stuck whenever i try to open a torrent file. i have tried various torrent websites and it always give a message like: ex 15.5 kb opened of 15.3 kb.. unknown time remaining. like it's opened more kb's than the actual size of the torrent file itself. again, i have tried many different torrent websites.. i can download pictures without problem, but whenever i try to download a torrent now.. it gets stuck.. it didn't do this last week.. firefox had an update last week and then another 1 the following day after and now i can't open torrent files anymore... HELP!
Flags: needinfo?(mmc)
Kurt, what version of Firefox are you on? This bug is specific to Firefox 32 Beta.
Flags: needinfo?(mmc)
Kurt, this bug was introduced in FF 32 beta 9. It will be fixed in FF 32 rc1, which is going to build today.
Monica, can we get a mochitest for this particularly major regression? I would be more confident in seeing this test run by buildbot instead of an external framework.
Flags: in-qa-testsuite?(hskupin) → needinfo?(mmc)
Henrik, there was actually an xpcshell test for this: https://hg.mozilla.org/integration/mozilla-inbound/rev/c8387e2ab335

I need to figure out why the callback wasn't reached in beta. Also, because so many of the safebrowsing checks depend on the presence of lists and responses from remote servers, the amount of testing we can do in mochitest and xpcshell is limited. That's why tests like https://bugzilla.mozilla.org/show_bug.cgi?id=1018161 are so important.
Flags: needinfo?(mmc)

Updated

3 years ago
Duplicate of this bug: 1058238
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #42)
> Henrik, there was actually an xpcshell test for this:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c8387e2ab335
> 
> I need to figure out why the callback wasn't reached in beta. 

The test clears the pref, but does not remove it. As per comment 9, removing will cause the NS_ENSURE_SUCCESS to return without calling the callback. (IIRC we disallowed this construct in the C++ coding guidelines for new code recently, exactly because it produces errors like this)

You could add a test with clearing vs resetting the pref, but I actually agree with Monica that this really has to be tested in a setup with network access. The buildbot tests mess with the exact same pref that caused the error because they don't want to access the network. This means that they're never really testing what the real build actually does.
My second paragraph should've read "clearing vs removing".
(In reply to Gian-Carlo Pascutto [:gcp] from comment #44)
> agree with Monica that this really has to be tested in a setup with network
> access. The buildbot tests mess with the exact same pref that caused the
> error because they don't want to access the network. This means that they're
> never really testing what the real build actually does.

Sounds good then. Would you mind to create a new bug in Mozilla QA / Mozmill Tests, which describes exactly what you want to have? You know better than me all the details to check. We can then work on a test with higher priority. Thanks.
Filed Bug 1058420 for more MozMill coverage.
Depends on: 1058420
Thanks, we will start working on a test soon.
Duplicate of this bug: 1012664

Updated

3 years ago
Duplicate of this bug: 1058361

Updated

3 years ago
Duplicate of this bug: 1058455

Updated

3 years ago
Duplicate of this bug: 1058461

Updated

3 years ago
Duplicate of this bug: 1058528

Updated

3 years ago
Duplicate of this bug: 1058553

Updated

3 years ago
Duplicate of this bug: 1058581
Duplicate of this bug: 1058616
I've verified in today's Fx32 RC, Fx33 and Fx34 that downloads of both signed and unsigned EXEs as well as other file types download correctly. The pref is no longer present in Fx32 as well. 

I'm marking verified. If there are other tests that need to be performed, please let me know and I'll do those ASAP. Thank you.
Status: RESOLVED → VERIFIED
status-firefox32: fixed → verified
status-firefox33: fixed → verified
status-firefox34: fixed → verified

Updated

3 years ago
Duplicate of this bug: 1058705

Updated

3 years ago
Duplicate of this bug: 1058694

Updated

3 years ago
Duplicate of this bug: 1058751

Comment 61

3 years ago
Hi Matt.

The latest fix worked for me (FF 32 beta, windows 7 64-bit).

Updated

3 years ago
Duplicate of this bug: 1059150

Updated

3 years ago
Duplicate of this bug: 1059422

Comment 64

3 years ago
Yup! I no longer have this issue, thanks Mozilla team!
(In reply to james.combs7 from comment #64)
> Yup! I no longer have this issue, thanks Mozilla team!

You're welcome, and I apologize for introducing the issue in the first place.

Comment 66

3 years ago
The issue is gone for me as well after 20140825202822. Thanks!
You need to log in before you can comment on or make changes to this bug.