1405166 - Remove system-disabled pref from Screenshots

Status: VERIFIED FIXED

(Firefox :: Screenshots, defect)

Description

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

No version update for this, we're going to import several small bugs (in parallel) and then bump the version.

This is a fix for this bug: https://github.com/mozilla-services/screenshots/issues/3549

This is an export of this PR: https://github.com/mozilla-services/screenshots/pull/3554

Comment 1

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Bug 1405166 - Remove system-disabled pref from Screenshots;

Review commit: https://reviewboard.mozilla.org/r/186094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186094/

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 8914814 [details]
Bug 1405166 - Remove system-disabled pref from Screenshots;

https://reviewboard.mozilla.org/r/186094/#review191194

There are still a bunch of in-tree references to this preference:

http://searchfox.org/mozilla-central/search?q=screenshots.system-disabled&case=true&path=

Please remove them as well.

Thanks

Comment 3

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Bug 1405166 - Update occurrences of system-disabled pref outside screenshots system addon;

Review commit: https://reviewboard.mozilla.org/r/186226/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186226/

Comment 4

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8914814 [details]
Bug 1405166 - Remove system-disabled pref from Screenshots;

Updated. Mind taking another look?

Unit tests are passing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3df879bfb25229628fd8370dd30ccf969dcae5d9

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8914966 [details]
Bug 1405166 - Update occurrences of system-disabled pref outside screenshots system addon;

https://reviewboard.mozilla.org/r/186226/#review191938

Comment 6

[Kris Maglione [:kmag]]

Comment on attachment 8914814 [details]
Bug 1405166 - Remove system-disabled pref from Screenshots;

https://reviewboard.mozilla.org/r/186094/#review191940

Comment 7

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 2 changesets with 6 changes to 6 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev 82078bee1e89 needs "Bug N" or "No bug" in the commit message.
remote: Jared Hirsch <[e-mail]>
remote: Update occurrences of system-disabled pref outside screenshots system addon r=kmag
remote: 
remote: MozReview-Commit-ID: 24PV53VM6te
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote

Comment 8

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8914966 [details]
Bug 1405166 - Update occurrences of system-disabled pref outside screenshots system addon;

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77d4d9eb87cd
Remove system-disabled pref from Screenshots; r=kmag
https://hg.mozilla.org/integration/autoland/rev/d15f945998db
Update occurrences of system-disabled pref outside screenshots system addon; r=kmag

Comment 10

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

This patch removes the `extensions.screenshots.system-disabled` pref that was being monitored by Telemetry. We're aiming to uplift to 57. Anything that needs to happen on the Telemetry side?

Comment 11

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8914814 [details]
Bug 1405166 - Remove system-disabled pref from Screenshots;

Approval Request Comment
[Feature/Bug causing the regression]:

Screenshots rollout was controlled by a shield study that flipped a pref. For users to exit the shield study without losing screenshots, we need this patch, which makes Screenshots ignore the pref.

[User impact if declined]:

Without uplift, users will remain in the shield study until this patch rides the trains to 58. Users would only lose screenshots if an unknown error were to occur with the shield recipe.

[Is this code covered by automated tests?]:

No

[Has the fix been verified in Nightly?]:

No

[Needs manual test from QE? If yes, steps to reproduce]: 

No

[List of other uplifts needed for the feature/fix]:

Both commits attached to bug 1405166

[Is the change risky?]:

Not risky

[Why is the change risky/not risky?]:

This patch removes a pref that was used to gradually roll out screenshots. Its removal ensures screenshots stays enabled for the release audience.

[String changes made/needed]:

None

Comment 12

[Georg Fritzsche [:gfritzsche]]

(In reply to Jared Hirsch [:_6a68] [:jhirsch] from comment #10)
> This patch removes the `extensions.screenshots.system-disabled` pref that
> was being monitored by Telemetry. We're aiming to uplift to 57. Anything
> that needs to happen on the Telemetry side?

No, that's all that is required.
Mark, is there anything needed on the pipeline side to remove user prefs from datasets like longitudinal?

Comment 13

[Mark Reid [:mreid]]

I don't think this will impact any of the existing datasets - in Longitudinal, prefs are stored as a string:string map, and in main_summary we don't have this pref at all.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/77d4d9eb87cd
https://hg.mozilla.org/mozilla-central/rev/d15f945998db

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8914814 [details]
Bug 1405166 - Remove system-disabled pref from Screenshots;

must fix, Beta57+

Comment 16

[Oriol Brufau [:Oriol]]

This reenabled the Screenshots thingie. It would have been nicer if this new extensions.screenshots.disabled pref was initialized to the value of the old extensions.screenshots.system-disabled

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/96eb7bfcdaf8
https://hg.mozilla.org/releases/mozilla-beta/rev/487b6f758af6

Comment 18

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Reproduced the initial behavior using  57.0a1 (2017-08-03). 
57.0b7 build1 (20171009192146) and 58.0a1 (2017-10-10) builds are verified fixed, using Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6.