Disable malware download protection in Firefox for Android 46

RESOLVED FIXED in Firefox 46

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Margaret, Assigned: Margaret)

Tracking

46 Branch
Firefox 47
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed, firefox47 fixed, fennec46+)

Details

Attachments

(1 attachment)

Assignee

Description

3 years ago
We don't want to ship malware download protection (bug 936041) until we have the notification in place (bug 1239094). Since that won't happen until 47, we should disable this feature on 46.
There's an implicit call here that we rather download malware rather than confuse the user. No judgement, just pointing out the tradeoff.
OS: Unspecified → Android
Hardware: Unspecified → All
See Also: → 1239094
Summary: Disabled malware download protection on Fx46 → Disable malware download protection in Firefox for Android 46
Version: unspecified → 46 Branch
Assignee

Comment 2

3 years ago
(In reply to Gian-Carlo Pascutto [:gcp] from comment #1)
> There's an implicit call here that we rather download malware rather than
> confuse the user. No judgement, just pointing out the tradeoff.

Correct, we discussed this point in the Android funnel meeting earlier this week.

NI to Barbara to confirm that decision.
Flags: needinfo?(bbermes)
Assignee

Updated

3 years ago
Assignee: nobody → margaret.leibovic
confirmed
Flags: needinfo?(bbermes)
Assignee

Comment 5

3 years ago
I still need to update the experiments config, but what do you think of this approach for setting gecko prefs?
Comment on attachment 8713804 [details]
MozReview Request: Bug 1241566 - Put malware download protection behind a switchboard flag. r=liuche

https://reviewboard.mozilla.org/r/32829/#review29823

With change to address always changing the pref.

::: mobile/android/chrome/content/browser.js:6974
(Diff revision 1)
> +            Services.prefs.setBoolPref("browser.safebrowsing.downloads.enabled", true);

What if a user turns off malware protection by toggling the pref - doesn't this turn it on again every time time Experiments gets init-ed?

I'm not sure how to check that - prefHasUserValue doesn't record if a user has toggled the pref back to the default value (which we're changing in the mobile.js part of this patch I think).
Attachment #8713804 - Flags: review?(liuche) → review+
Assignee

Comment 7

3 years ago
(In reply to Chenxia Liu [:liuche] from comment #6)
> Comment on attachment 8713804 [details]
> MozReview Request: Bug 1241566 - Put malware download protection behind a
> switchboard flag. r=liuche
> 
> https://reviewboard.mozilla.org/r/32829/#review29823
> 
> With change to address always changing the pref.
> 
> ::: mobile/android/chrome/content/browser.js:6974
> (Diff revision 1)
> > +            Services.prefs.setBoolPref("browser.safebrowsing.downloads.enabled", true);
> 
> What if a user turns off malware protection by toggling the pref - doesn't
> this turn it on again every time time Experiments gets init-ed?

Good catch.

> I'm not sure how to check that - prefHasUserValue doesn't record if a user
> has toggled the pref back to the default value (which we're changing in the
> mobile.js part of this patch I think).

I don't think we need to worry about this case - this is just an about:config pref, and we don't make any guarantees of service over that.

But you know what... this patch is actually bad because it doesn't handle reverting the pref back to the mobile.js default when the experiment is over. I think I actually need to rethink this set the pref to false if the experiment *isn't* active.
Assignee

Comment 8

3 years ago
Comment on attachment 8713804 [details]
MozReview Request: Bug 1241566 - Put malware download protection behind a switchboard flag. r=liuche

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32829/diff/1-2/
Assignee

Comment 9

3 years ago
I decided to update the patch to clear the prefs before we check the active experiments. This should take care of resetting the prefs if the experiment is disabled.

I don't think we should try to support users changing this pref in about:config. It's an implementation detail that they can change this in about:config, they shouldn't be manually enabling/disabling malware protection.

Some people may disagree with me on this, but I see about:config as a developer/tester tool, not a tool for end users.
Flags: needinfo?(liuche)
Why not use default prefs branch for managing these experiments? This is how we set prefs via distributions.
Assignee

Comment 11

3 years ago
(In reply to Mark Finkle (:mfinkle) from comment #10)
> Why not use default prefs branch for managing these experiments? This is how
> we set prefs via distributions.

This is the right call, I forgot about default prefs. I'm going to update the patch to do this, and it will solve the problems mentioned above.
Flags: needinfo?(liuche)
Assignee

Comment 12

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/6599b14546f1a62e7089e9b8b404799db54b5da0
Bug 1241566 - Put malware download protection behind a switchboard flag. r=liuche
Assignee

Comment 13

3 years ago
Comment on attachment 8713804 [details]
MozReview Request: Bug 1241566 - Put malware download protection behind a switchboard flag. r=liuche

Approval Request Comment
[Feature/regressing bug #]: Bug 936041.

[User impact if declined]: Feature will be enabled that we did not intend to ship.

[Describe test coverage new/current, TreeHerder]: Tested locally to confirm this disables the feature. 

[Risks and why]: Low-risk, pref change to disable feature. There's more risk than normal because I also added code to control this pref via switchboard, but if that logic is broken, the feature would just stay disabled. So it wouldn't introduce a new problem.

[String/UUID change made/needed]: None.
Attachment #8713804 - Flags: approval-mozilla-aurora?

Comment 14

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6599b14546f1
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment on attachment 8713804 [details]
MozReview Request: Bug 1241566 - Put malware download protection behind a switchboard flag. r=liuche

Holding back this feature until 47. Please uplift to aurora.
Attachment #8713804 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.