1066671 - Regression: Downloading is broken

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Currently downloading is broken in Nightly based on mozilla-central.

Last good revision: 152ef25e89ae (2014-09-10)
First bad revision: 98ea98c8191a (2014-09-11)

Pushlog:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=152ef25e89ae&tochange=98ea98c8191a

Comment 1

[Aaron Train [:aaronmt]]

Last good revision: 02fbb6ada9cb
First bad revision: bc7deafdac4b
Pushlog:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=02fbb6ada9cb&tochange=bc7deafdac4b

bf52063ed95f	Wes Johnston — Bug 1058150 - Use restricted profiles for guest mode. r=mfinkle

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: rprofiles-fixup v.1 (patch 0)

This patch fixes some stuff that Wes missed the first time. Well, he had a patch that fixed it but it's not what landed.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: rprofiles-dload-bug v0.1

This patch fixes the download issue. The trouble starts with the fact that we keep flipping boolean contexts.

1. Android state values: "no_download_files" means the false allows downloads
2. nsIPCS.isAllowed(DOWNLOADS): means true allows downloads. We already do a boolean flip and I added a comment making it more explicit.
3. nsIPCS.GetBlockFileDownloadsEnabled(...) returns the blocked state meaning false allows downloads. This is the bug. We were not flipping the boolean. This patch does that.

I am now wondering if we should change nsIPCS.isAllowed to nsIPCS.isBlocked so we can stop worrying about flipping booleans.

Thoughts?

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 8489049 [details] [diff] [review]
rprofiles-dload-bug v0.1

I'd be fine with the isBlocked switch. I noticed this when I was doing it, but decided I just hated BlockFileDownloadEnabled so I stubbornly didn't change my interface :) Not super attached though.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Wes - any reason we shouldn't review the patch0 as well? As for the change to isBlocked, I'll file a followup bug.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/fx-team/rev/8201c5f8e7c8
https://hg.mozilla.org/integration/fx-team/rev/f403a0e6db85

And I added the lost changes to the test file too. At the same time, I added a simple check for pc.blockFileDownloadsEnabled which would have caught this bug.
https://hg.mozilla.org/integration/fx-team/rev/db8fb4f403a0

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8201c5f8e7c8
https://hg.mozilla.org/mozilla-central/rev/f403a0e6db85
https://hg.mozilla.org/mozilla-central/rev/db8fb4f403a0

Comment 9

[Cristina Madaras, QA [:CristinaM]]

Verified as fixed in
Build: Firefox for Android 35.0a1 (2014-09-17)
Device: Asus Transformer Pad TF300T (Android 4.2.1)