Replace data choices infobar with privacy policy in a background tab on first run

VERIFIED FIXED in Firefox 55

Status

()

VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: pdol, Assigned: dao)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

User Story

Currently, 60 seconds into the first run of Firefox we open a bottom infobar that says, "Firefox automatically sends some data to Mozilla so that we can improve your experience. [Choose What I Share]". Based on the results of our funnelcake 100, we want to prevent that infobar from showing on first run (or at any other time) and instead open a second tab (the first being the wow moment + account sign-in page) in the background that loads https://www.mozilla.org/privacy/firefox/.  This should go out in v55 unless risk is unacceptable.

The implementation should be based on bug 1324049.

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

2 years ago
Assignee: nobody → dao+bmo
(Assignee)

Comment 1

2 years ago
Posted patch patch (obsolete) — Splinter Review
This breaks reftests, crashtests and marionette:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=94e8c6859aac99aa65e84c063b09b546c17e2ac5

Not sure where the prefs for these test suites are set. According to <https://developer.mozilla.org/en-US/docs/Mozilla/QA/Automated_testing#Need_to_set_preferences_for_test-suites>, modifying testing/mozbase/mozprofile/mozprofile/profile.py should do the trick, but this didn't seem to work here.
(Assignee)

Comment 2

2 years ago
Posted patch patch (obsolete) — Splinter Review
This fixes reftests and crashtests, marionette remains broken:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a37a1920787d0cb48f0bb97e330f3b8ed7099cd6
Attachment #8870785 - Attachment is obsolete: true
(Assignee)

Comment 3

2 years ago
Comment on attachment 8870828 [details] [diff] [review]
patch

Henrik, where do I need to set datareporting.policy.dataSubmissionPolicyBypassNotification=true for marionette to pick this up in automation? testing/mozbase/mozprofile/mozprofile/profile.py as documented on MDN doesn't seem to work.
Attachment #8870828 - Flags: feedback?(hskupin)
There are currently two places because we are in a transition:

https://dxr.mozilla.org/mozilla-central/source/testing/marionette/server.js (Recommended prefs)
https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette_driver/geckoinstance.py

For the latter it depends on if his pref is only for firefox or gecko in general. So pick the correct class.
(Assignee)

Comment 5

2 years ago
Comment on attachment 8870828 [details] [diff] [review]
patch

(In reply to Henrik Skupin (:whimboo) from comment #4)
> There are currently two places because we are in a transition:
> 
> https://dxr.mozilla.org/mozilla-central/source/testing/marionette/server.js
> (Recommended prefs)
> https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/
> marionette_driver/geckoinstance.py
> 
> For the latter it depends on if his pref is only for firefox or gecko in
> general. So pick the correct class.

Thanks. The pref lives in toolkit so "gecko" probably makes more sense.
Attachment #8870828 - Flags: feedback?(hskupin)
(Assignee)

Comment 6

2 years ago
(In reply to Henrik Skupin (:whimboo) from comment #4)
> There are currently two places because we are in a transition:
> 
> https://dxr.mozilla.org/mozilla-central/source/testing/marionette/server.js
> (Recommended prefs)
> https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/
> marionette_driver/geckoinstance.py
> 
> For the latter it depends on if his pref is only for firefox or gecko in
> general. So pick the correct class.

Wait, both of these files were already setting datareporting.policy.dataSubmissionPolicyBypassNotification=true. That pref appears to work as expected for reftests and mochitests but not for marionette (see https://treeherder.mozilla.org/#/jobs?repo=try&revision=a37a1920787d0cb48f0bb97e330f3b8ed7099cd6). Any other clue what could be going wrong here?
Flags: needinfo?(hskupin)
The test which is failing here, resets the profile and restarts Firefox. So all set preferences are getting cleared. Gijs, can most likely help here given that he initially wrote this test, and could say what to do with it. 

Keep in mind that this is not a usual Marionette unit test, but a browser test which is folded into the Mn test suite, due to a missing subtest feature yet. The test is located here:

https://dxr.mozilla.org/mozilla-central/source/browser/components/migration/tests/marionette/test_refresh_firefox.py
Flags: needinfo?(hskupin) → needinfo?(gijskruitbosch+bugs)

Comment 8

2 years ago
There isn't an easy way to fix this that I can see.

Perhaps the most straightforward option I can see would be to change the "marionette." hardcoded prefix for MOZ_MARIONETTE_PREF_STATE_ACROSS_RESTARTS, and to then include the relevant pref in one of the ones marionette sets ASAP in the process' lifetime, here: https://dxr.mozilla.org/mozilla-central/rev/f81bcc23d37d7bec48f08b19a9327e93c54d37b5/browser/components/migration/tests/marionette/test_refresh_firefox.py#378-388
However, I don't know if that will come up to any limits on the length of environment variables on the different platforms on which we run the test.

A more correct fix would be avoiding reshowing the notification bar or the page if the user had already seen it and/or it was disabled on the old profile. That would have to be fixed in FirefoxProfileMigrator.js, but we still don't have a general way of migrating prefs and so fixing that is non-trivial, I think.

Another hacky thing you could do is avoiding the notification bar/page codepath when session restore has restored something (which is the case here, and pretty much guarantees the user has already seen it anyway, or will have it restored in a new tab), or changing the test to simply close/remove the tab you're adding.
Flags: needinfo?(gijskruitbosch+bugs)
Hey Dao and Gijs,

Thanks for helping the onboarding team with these bugs. Dao, what do you think is the best way to proceed with this fix?

We planned on landing this in 55, and ideally, we would start the QA testing for this bug starting tomorrow.
Flags: needinfo?(dao+bmo)
Target Milestone: --- → Firefox 55
(Assignee)

Updated

2 years ago
Attachment #8870828 - Attachment is obsolete: true
Flags: needinfo?(dao+bmo)

Comment 11

2 years ago
mozreview-review
Comment on attachment 8872958 [details]
Bug 1367076 - Set datareporting.policy.firstRunURL by default to replace data choices infobar with privacy policy in a background tab on first run.

https://reviewboard.mozilla.org/r/144500/#review148408
Attachment #8872958 - Flags: review?(gijskruitbosch+bugs) → review+
Hey Dao,

Can you commit these changes to m-c? Code Freeze is tomorrow, June 5th. If you don't have commit access, RyanVM can you please help us do this?
Flags: needinfo?(ryanvm)
Flags: needinfo?(dao+bmo)

Comment 13

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ffd92f10ba94
Set datareporting.policy.firstRunURL by default to replace data choices infobar with privacy policy in a background tab on first run. r=Gijs
Flags: needinfo?(ryanvm)
Flags: needinfo?(dao+bmo)
https://hg.mozilla.org/mozilla-central/rev/ffd92f10ba94
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
(Assignee)

Comment 15

2 years ago
(In reply to Nicole Yee (:nicoleyee) from comment #12)
> Hey Dao,
> 
> Can you commit these changes to m-c? Code Freeze is tomorrow, June 5th. If
> you don't have commit access, RyanVM can you please help us do this?

Monday was a bank holiday in Germany, and I was still waiting for gfritzsche's review. I'm leaving the request in place in case he has something to add.

Comment 16

2 years ago
mozreview-review
Comment on attachment 8872958 [details]
Bug 1367076 - Set datareporting.policy.firstRunURL by default to replace data choices infobar with privacy policy in a background tab on first run.

https://reviewboard.mozilla.org/r/144500/#review150074

This looks good.

While the Telemetry change seems trivial, this is critical to our Telemetry reporting.
Are you planning to have a QA pass here or do we need to?
Attachment #8872958 - Flags: review?(gfritzsche) → review+
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 17

2 years ago
(In reply to Georg Fritzsche [:gfritzsche] from comment #16)
> While the Telemetry change seems trivial, this is critical to our Telemetry
> reporting.
> Are you planning to have a QA pass here or do we need to?

Forwarding this question to Peter.
Flags: needinfo?(dao+bmo) → needinfo?(pdolanjski)
(Reporter)

Comment 18

2 years ago
Nicole, can you follow up with Soft vision to ensure QA coverage?
Flags: needinfo?(pdolanjski) → needinfo?(nyee)
I can verify that https://www.mozilla.org/privacy/firefox/ loads as a background tab instead of the infobar "Firefox automatically sends some data to Mozilla so that we can improve your experience. [Choose What I Share]".
Status: RESOLVED → VERIFIED
Flags: needinfo?(nyee)

Comment 20

2 years ago
mozreview-review
Comment on attachment 8872958 [details]
Bug 1367076 - Set datareporting.policy.firstRunURL by default to replace data choices infobar with privacy policy in a background tab on first run.

https://reviewboard.mozilla.org/r/144500/#review179472

::: testing/mozbase/mozprofile/mozprofile/profile.py:357
(Diff revision 1)
>          # Don't warn when exiting the browser
>          'browser.warnOnQuit': False,
>          # Don't send Firefox health reports to the production server
>          'datareporting.healthreport.documentServerURI': 'http://%(server)s/healthreport/',
> +        # Skip data reporting policy notifications
> +        'datareporting.policy.dataSubmissionPolicyBypassNotification': True,

Dao, FYI this is not the place to add preferences anymore. I don't know how many test harnesses still rely on those, but for at least Marionette we moved them all out into geckoinstance.py, and marionette.js.

We should finally strip out all those prefs from mozprofile.

Updated

a year ago
Depends on: 1417832

Updated

a year ago
No longer depends on: 1417832
You need to log in before you can comment on or make changes to this bug.