1411797 - Turn off activity stream for 57

Status: RESOLVED WONTFIX

(Firefox :: New Tab Page, enhancement)

Description

[Ed Lee :Mardak]

[Tracking Requested - why for this release]:

Depending on the outcome of the shield study bug 1410535 and other decisions, we might need to turn off activity stream and show the refreshed Tiles about:newtab from bug 1411419 instead.

Comment 1

[Marcia Knous [:marcia]]

Tracking 57+ to keep this on the radar.

Comment 2

[Ed Lee :Mardak]

With various tests fixed and others skipped, seems like it's the normal level of test failures with activity stream enabled and disabled:
https://treeherder.mozilla.org/#/jobs?repo=try&fromchange=a835467c518361e027fe9498a6a4a80d42fc1b23&tochange=23d3488ab58ec272e068d546be01fba3df6bbf7a

Comment 3

[Ed Lee :Mardak]

Attachment: Bug 1411797 - Turn off Activity Stream for 57

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

Comment 4

[Fischer [:Fischer]]

Based on the meeting, this bug should also handle the startup profile migration (currently being turned off)

Comment 5

[Fischer [:Fischer]]

Attachment: Bug 1411797 - Revert Bug 1403402

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

Comment 6

[Fischer [:Fischer]]

(In reply to Fischer [:Fischer] from comment #5)
> Created attachment 8924111 [details]
> Bug 1411797 - Revert Bug 1403402
> 
> Review commit: https://reviewboard.mozilla.org/r/195344/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/195344/
This patch simply reverts Bug 1403402 in case that AS is off and we still want to prompt migration wizard for a fresh install during the 1st FF startup.
Basically this is very low risky for uplifting in the current time frame, because it just rolls back to the situation before Bug 1403402, which is the previous FF behavior.

However There is some notice.
This patch will make it ALWAYS prompt migration wizard for a fresh install during the 1st FF startup.
So after uplifting, if we wanted to turned on AS for some new user for experiments in 57, we would encounter the issue that we prompt migration wizard during the 1st startup and display the manual migration option on AS page again.

At the moment of startup, it decides the case that should it prompt migration wizard for a fresh install during the 1st FF startup at [1].
However, at that moment we are unable to get AS-enabled pref with `Preferences::GetBool("browser.newtabpage.activity-stream.enabled", false);` because the prefs are not yet initiated. We just returned at `InitStaticMembers` is false[2]. As a result cannot flip the `gDoMigration` flag based on the AS enabling state.

If we wanted to preserve the flexibility, we might need to look into another approach.

[1] http://searchfox.org/mozilla-central/rev/1ebd2eff44617df3b82eea7d2f3ca1b60cc591a0/toolkit/xre/nsAppRunner.cpp#2581
[2] http://searchfox.org/mozilla-central/rev/aa1343961fca52e8c7dab16788530da31aab7c8d/modules/libpref/Preferences.cpp#4825

Comment 7

[Ed Lee :Mardak]

mikedeboer, if we turn off browser.library.activity-stream.enabled as part of turning off activity stream (still undecided from a product side as the library view does exist somewhat independent of the about:newtab/about:home code), is there anything to pay particular attention to? Note that this would be a 57-mozilla-beta-only change.

Comment 8

[:Gijs (he/him)]

(In reply to Ed Lee :Mardak from comment #7)
> mikedeboer, if we turn off browser.library.activity-stream.enabled as part
> of turning off activity stream (still undecided from a product side as the
> library view does exist somewhat independent of the about:newtab/about:home
> code), is there anything to pay particular attention to? Note that this
> would be a 57-mozilla-beta-only change.

To be clear, this would affect 57 release, and I *think* we've merged beta to the mozilla-release repo at this point because we're past last-non-RC-beta (but maybe I'm wrong and that's not until later?).

I mean, the obvious thing is that we should check that the library popup works without the AS items. It should, but testing would be a Good Idea. I'll let Mike reply to see if there's anything else.

Comment 9

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs (slow, PTO recovery mode) from comment #8)
> I mean, the obvious thing is that we should check that the library popup
> works without the AS items. It should, but testing would be a Good Idea.
> I'll let Mike reply to see if there's anything else.

This, but the Highlights are powered by queries that do not rely on the AS backend, just on NewTabUtils.jsm gershizzle, so I'm not expecting weird fallout from this pref flip. A wee bit of QA coverage in this area wouldn't hurt, of course.

I'd like to throw in an aside to this bug; as someone who is not involved directly in the AS team and decision making, this last-minute disabling of a big feature takes me by surprise and I do hope that everyone involved is aware of the fact that it's a far from _free_ an operation.
Additionally, I quite enjoyed the AS experience TBH... it offers so much more than our whacky about:home and about:newtab ruins.

Comment 10

[Ed Lee :Mardak]

To be clear, the decision has not been made to turn off activity stream for 57. People outside of the team are making the decision, and we're trying to provide them with actual data and actual choices (i.e., turning off activity stream should "just work" without a broken / outdated result that would require even more engineering late late in beta cycle).

Comment 11

[Nicole Yee (:nicoleyee)]

Cristian can you please provide some testing (In reply to Mike de Boer [:mikedeboer] from comment #9)
> (In reply to :Gijs (slow, PTO recovery mode) from comment #8)
> > I mean, the obvious thing is that we should check that the library popup
> > works without the AS items. It should, but testing would be a Good Idea.
> > I'll let Mike reply to see if there's anything else.
> 
> This, but the Highlights are powered by queries that do not rely on the AS
> backend, just on NewTabUtils.jsm gershizzle, so I'm not expecting weird
> fallout from this pref flip. A wee bit of QA coverage in this area wouldn't
> hurt, of course.

Cristan can you provide this QA support for us?

Comment 12

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

For this feature Iulia Cristescu is the feature owner. She will provide the QA support.

Comment 13

[Ed Lee :Mardak]

A final decision has been made to ship 57 with activity stream.