1322731 - Replace default tiles for the funnelcake in feb 2017

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[:Gijs (he/him)]

This should be behind a pref... the new defaults:

https://www.google.com/
https://www.youtube.com/
https://www.facebook.com/
https://www.wikipedia.org/
https://www.yahoo.com/
https://www.amazon.com/

Comment 1

[:Gijs (he/him)]

Verdi, can you sanity-check the list in comment #0?


There's two basic ways we can try to get this done:

1) hardcode a list somewhere and override the results from the directorylinksprovider (which comes from the tiles server) based on a pref.
2) send funnelcake info to the directorylinksprovider and teach the directorylinksprovider service to be able to have a separate set of results for the funnelcake. Right now it uses update channels (client-side, we use UpdateUtils.UpdateChannel), listed here: https://github.com/mozilla/onyx/blob/master/onyx/api/v3.py#L17 .


I'm leaning towards (1) in order to minimize the number of dependencies and therefore risk, given that we want to have this done by the first week of January. On the flip side, (2) would give us more flexibility in the future. On the flip side of that, AFAICT the tiles service is not very useful anymore these days (we still fetch tiles once a day but we never send 'enhanced' tiles and so for mature profiles we're basically making 1 useless https request a day to fetch this data we never use) and so maybe we should think about removing that code and shutting it down, which makes (2) a lot less attractive. Dolske, thoughts?

Comment 2

[Verdi [:verdi] Best to slack me]

(In reply to :Gijs Kruitbosch from comment #1)
> Verdi, can you sanity-check the list in comment #0?

That's the list. Looks good.

Comment 3

[Justin Dolske [:Dolske]]

(In reply to :Gijs Kruitbosch from comment #1)

> There's two basic ways we can try to get this done:
> 
> 1) hardcode a list somewhere and override the results from the
> directorylinksprovider (which comes from the tiles server) based on a pref.
> 2) send funnelcake info to the directorylinksprovider and teach the
> directorylinksprovider service to be able to have a separate set of results
> for the funnelcake. Right now it uses update channels (client-side, we use
> UpdateUtils.UpdateChannel), listed here:
> https://github.com/mozilla/onyx/blob/master/onyx/api/v3.py#L17 .
> 
> 
> I'm leaning towards (1) in order to minimize the number of dependencies and
> therefore risk

Simplicity seems best to me.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1322731 - allow using different default tiles,

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

Comment 5

[:Gijs (he/him)]

I've added a screenshot on mozreview, Michael, to make it easy to review.

To run with these changes, we'd need the following pref change:

 // directory tiles download URL
-pref("browser.newtabpage.directory.source", "https://tiles.services.mozilla.com/v3/links/fetch/%LOCALE%/%CHANNEL%");
+pref("browser.newtabpage.directory.source", "chrome://browser/content/newtab/alternativeDefaultSites.json");

Open question: the directoryId values. I don't know if I'm allowed to just make those up, or how else I would make that work. Marina or Tim, do you know?

Comment 6

[:Gijs (he/him)]

(oh, and just so we're clear, this is without any further styling changes to about:newtab, this is just the different about:newtab URLs, but I added metadata rather than having about:newtab start requesting all these sites on startup to get thumbnails...)

Comment 7

[Verdi [:verdi] Best to slack me]

Comment on attachment 8821502 [details]
Bug 1322731 - allow using different default tiles,

https://reviewboard.mozilla.org/r/100792/#review101332

Comment 8

[Verdi [:verdi] Best to slack me]

Comment on attachment 8821502 [details]
Bug 1322731 - allow using different default tiles,

https://reviewboard.mozilla.org/r/100794/#review101334

Comment 9

[Dão Gottwald [::dao]]

Comment on attachment 8821502 [details]
Bug 1322731 - allow using different default tiles,

https://reviewboard.mozilla.org/r/100794/#review101484

Will we need to add extra instrumentation to evaluate the usage of these tiles or will this happen automatically through existing mechanisms?

Comment 10

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #9)
> Comment on attachment 8821502 [details]
> Bug 1322731 - allow using different default tiles,
> 
> https://reviewboard.mozilla.org/r/100794/#review101484
> 
> Will we need to add extra instrumentation to evaluate the usage of these
> tiles or will this happen automatically through existing mechanisms?

The directoryIds will be used for reporting, AIUI, but as noted in comment #5 I don't know if the ones I specified work or if we need server changes to make them work. I'm kind of assuming that nothing disastrous will happen if we submit stuff to the tiles server with 'invalid' IDs because for obvious security reasons it should already be resilient to 'bogus' stuff being submitted... Depending on what Marina and Tim tell us about this we may want to add telemetry or other mechanisms of our own, though I would prefer to use the existing one if possible.

Comment 11

[:Gijs (he/him)]

Talked to Marina and Nan Jiang on IRC, and this should be good to go.

In principle there should be data available through the "Tiles" dataset on sql.tmo.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdcf8d6a9572
allow using different default tiles, r=dao,verdi

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cdcf8d6a9572

Comment 14

[:Gijs (he/him)]

Comment on attachment 8821502 [details]
Bug 1322731 - allow using different default tiles,

Approval Request Comment
[Feature/Bug causing the regression]: funnelcake for bug 1322718
[User impact if declined]: impact to the funnelcake
[Is this code covered by automated tests?]: the new tab page is covered by automated tests, but this change is behind a pref.
[Has the fix been verified in Nightly?]: no, because it's behind a pref
[Needs manual test from QE? If yes, steps to reproduce]: yes, but we'll need a custom build. Verification is easy: on the custom build, on a clean profile with no imports from another browser (e.g. because Firefox was the default) we'll show different default tiles (see comment #0).
[List of other uplifts needed for the feature/fix]: the other deps for bug 1322718 are related, but not strictly necessary.
[Is the change risky?]: No.
[Why is the change risky/not risky?]:  it's completely behind a pref so will only affect the funnelcake. We'll ship an extra file, which is JSON and should be highly compressible, so the only minor impact to release is potentially a few byte increase in filesize
[String changes made/needed]: No.

Comment 15

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8821502 [details]
Bug 1322731 - allow using different default tiles,

OK to uplift, this might not make it to beta 12 - if not, beta 13 next mon/tues.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bac566c43bc4

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/a651c8af139d

Comment 18

[Andrei Vaida [:avaida]]

Setting the flag for verification, description available in Comment 0 and Comment 5.