1464019 - Snippets can no longer be turned off when browser.newtabpage.activity-stream.asrouterExperimentEnabled is true

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P2)

Description

[Ciprian Muresan [:cmuresan], Ecosystem QA]

Attachment: snippets not disabled.gif

[Affected versions]:
- Nightly 62.0a1 Build ID 20180522220730
- Beta 61.0b7    Build ID 20180521110957

[Affected Platforms]:
- All Windows
- All Mac
- All Linux

[Prerequisites]:
- Set "browser.newtabpage.activity-stream.feeds.snippets" to "true".
- Set "browser.onboarding.notification.finished" to "true".
- Set "browser.newtabpage.activity-stream.asrouterExperimentEnabled" to true.

[Steps to reproduce]:
1. Open a new tab and click the Gearwheel button.
2. Click the Snippets checkbox.
3. Open a New Tab and observe the bottom part of the page.

[Expected results]:
- Snippets are no longer displayed.

[Actual results]:
- Snippets are still displayed.

[Notes]:
- Attached a screen recording of the issue.

Comment 1

[Kate Hudson :k88hudson]

Ok, looks like flipping the snippets pref in about:preferences will trigger the experiment to be turned off, so this will not need an uplift. However, we should still fix this for 62

Comment 2

[Ciprian Muresan [:cmuresan], Ecosystem QA]

@kate, are we sure this is correct? Right now if we turn off snippets from about:preferences#home, they will NOT be turned off and will still be displayed on subsequent new tabs. Is this the correct behavior?

Comment 3

[Kate Hudson :k88hudson]

If our targeting parameters include that:
   browser.newtabpage.activity-stream.feeds.snippets must be true

and a user in the experiment who is seeing AS Router messages turns snippets off in about:preferences (i.e. sets browser.newtabpage.activity-stream.feeds.snippets to false), according to jkerim, they will get immediately dropped from the cohort. That means browser.newtabpage.activity-stream.asrouterExperimentEnabled would be flipped to false, and the snippets would disappear.

If this is true, the result is that:
- people in control for 61 are unaffected
- people in the experiment group are unaffected
- only people who manually flip their experiment pref would be effected 

That being said I appreciate this is kind of hard to verify/depends on shield targeting, so I'm also happy to just fix this and uplift it if we would be more comfortable with that

Comment 4

[Kate Hudson :k88hudson]

What's your opinion Ciprian?

Comment 5

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4172

Comment 6

[Kate Hudson :k88hudson]

I think I'm just going to fix this / uplift just in case.

Comment 7

[[github robot]]

Commits pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/fef272f2b40295de03cf2d075394824ddc2b651d
Bug 1464019 - Turn off asrouter snippets when snippets.feed is is false

https://github.com/mozilla/activity-stream/commit/74225d53b3dfbd6239a408e6882afd1a4cfffe4b
Merge pull request #4172 from k88hudson/bug1464019

Bug 1464019 - Turn off asrouter snippets when snippets.feed is false

Comment 8

[Ciprian Muresan [:cmuresan], Ecosystem QA]

@Kate, I didn't know we can drop users out of an experiment that easily. Personally, I think we're better off being safe rather than being sorry.

Comment 9

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4182

Comment 10

[Kate Hudson :k88hudson]

Attachment: Bug 1464019 - Turn off asrouter snippets when snippets.feed is is false

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

Comment 11

[Ed Lee :Mardak]

Comment on attachment 8981937 [details]
Bug 1464019 - Turn off asrouter snippets when snippets.feed is is false

https://reviewboard.mozilla.org/r/247946/#review254086

Comment 12

[Kate Hudson :k88hudson]

Comment on attachment 8981937 [details]
Bug 1464019 - Turn off asrouter snippets when snippets.feed is is false

Approval Request Comment
[Feature/Bug causing the regression]: AS Router snippets cannot be turned off by users if they are added to the experiment
[User impact if declined]: Experiment won't be able to be run
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Not yet, but it has been in AS github code base / run locally for two days
[Needs manual test from QE? If yes, steps to reproduce]: try turning off snippets from the about:preferences#home page while experiment pref (browser.newtabpage.activity-stream.asrouterExperimentEnabled) is on. snippets should disappear.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: feature is behind a pref
[String changes made/needed]: none

Comment 13

[Ciprian Muresan [:cmuresan], Ecosystem QA]

The issue was fixed by bug 1465172. 

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d7379d70b96374656b0102d8dc329f689a6c4872&tochange=609a288fe314039c44af39b4a1173f60a058a9b2

Comment 14

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have verified that the issue is no longer reproducible on the latest Nightly 62.0a1 (Build ID 20180603221731) on Windows 10 x64, Mac 10.13.4 and Ubuntu 16.04 x64. Snippets are no longer displayed on subsequent tabs after they are disabled when browser.newtabpage.activity-stream.asrouterExperimentEnabled is true.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8981937 [details]
Bug 1464019 - Turn off asrouter snippets when snippets.feed is is false

Verified fixed on Nightly, let's take this in today's 61.0b11 build as well.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/72fbfbc22b7b

Comment 17

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have verified that the issue is no longer reproducible on the latest Firefox Beta 61.0b11 (Build ID 20180604143143) on Windows 10 x64, Mac 10.13.5 and Arch Linux x64.