1140185 - Suggested Tiles: v1.0 Sponsored Release (FX 40)

Status: RESOLVED INCOMPLETE

(Content Services Graveyard :: Tiles, defect)

Description

[Ed Lee :Mardak]

Tracking bug for suggested tiles functionality that won't be part of the initial feature.

Comment 1

[Ed Lee :Mardak]

Similar to bug 1120311, which now focuses on getting unpaid/affiliate suggested tiles, this bug is for suggested tiles changes to work with paid/sponsored suggested tiles, e.g., messaging explaining paid content, not showing more than one sponsored tile.

Comment 2

[Kevin Ghim]

Goes to Aurora: 2015-03-30
Goes to Beta:   2015-05-11
Release Date:   2015-06-30

Comment 3

[Ed Lee :Mardak]

Adding these queries for easier reference:

fixed on 40 waiting to be uplifted to 39:
https://bugzilla.mozilla.org/buglist.cgi?f1=blocked&o1=equals&v1=1140185&f2=cf_status_firefox40&o2=anywords&v2=fixed+verified&o3=nowords&f3=cf_status_firefox39&v3=fixed+verified

fixed on 40 and uplifted to 39:
https://bugzilla.mozilla.org/buglist.cgi?f1=blocked&o1=equals&v1=1140185&f2=cf_status_firefox40&o2=anywords&v2=fixed+verified&o3=anywords&f3=cf_status_firefox39&v3=fixed+verified

Comment 4

[Ed Lee :Mardak]

These 11 bugs landed on 41 and need to be uplifted to 40 (and later to 39):

https://bugzilla.mozilla.org/buglist.cgi?f1=blocked&o1=equals&v1=1140185&f2=cf_status_firefox41&o2=anywords&v2=fixed+verified&o3=nowords&f3=cf_status_firefox40&v3=fixed+verified

That list currently matches the output:

git log gecko/aurora..gecko/fx-team --format='format:pick %h %s - %an' --reverse browser/{base/content{,/test},themes/shared}/newtab browser/{locales/en-US/chrome/browser/newTab.*,modules/DirectoryLinksProvider.jsm,docs/DirectoryLinksProvider.rst} toolkit/modules/NewTabUtils.jsm testing/profiles/prefs_general.js | grep -vE '(2bef2fe|6dc67d0|1160844|1158208|1157810|1166465|1160771)'

> pick 85b4893 Bug 1138818 - Part 1 - Onboarding UI without the tile images. r=adw - Marina Samuel
> pick e886493 Bug 1138818 - Part 2 - Add onboarding images. r=adw - Marina Samuel
> pick 3f7b612 Bug 1138818 - Part 3 - Change to localizable strings.  r=adw - Marina Samuel
> pick bdb9318 Bug 1160369 - Allow for sponsored suggested tiles [r=adw] - Ed Lee
> pick 4bafce9 Bug 1165386 - Always show SPONSORED for any type of sponsored tiles and SUGGESTED for other suggested [r=adw] - Ed Lee
> pick a661396 Bug 1165430 - Switch unlocalizable "Example Title" to "mozilla.org" [r=adw] - Ed Lee
> pick 33263c6 Bug 1139496 - Allow server provided explanation / ad group name to be displayed on Suggested Tiles [r=adw, f=bsmedberg] - Marina Samuel
> pick c60814f Bug 1166552 - browser_newtab_reportLinkAction.js fails on timeout [r=adw] - Ed Lee
> pick 99ab481 Bug 1159884 - Implement inadjacency with a hardcoded list of hashed sites [r=adw] - Maxim Zhilyaev
> pick 8c3e645 Bug 1165525 - Onboarding messaging poorly words how to turn off suggestions by implying blank is the only way [r=adw] - Ed Lee
> pick 0e1dc05 Bug 1159884 - Inadjacency documentation [f=bsmedberg] - Ed Lee
> pick 620576d Bug 1145428 - Suggested Tiles pins (becomes a user history tile after end time) [r=adw] - Maxim Zhilyaev
> pick f955a0a Bug 1167805 - Not enough padding for page title causing pin icon to overlap [r=adw] - Ed Lee
> pick f7251f5 Bug 1167243 - Intermittent browser_newtab_enhanced.js | Suggested for {'Web Education'|'Technology'} enthusiasts  [r=adw] - Marina Samuel

Comment 5

[Ed Lee :Mardak]

emtwo/maksik, this query should be all the bugs that need to be uplifted to beta 39:

https://bugzilla.mozilla.org/buglist.cgi?f1=blocked&o1=equals&v1=1140185&f2=cf_status_firefox39&o2=nowords&v2=fixed+verified&j3=OR&f3=OP&f4=cf_status_firefox40&o4=anywords&v4=fixed+verified&f5=cf_status_firefox41&o5=anywords&v5=fixed+verified

Can you double check if any of those patches need some special handling for uplift (i.e., en-US only changes).

Comment 6

[Marina Samuel [:emtwo]]

Attachment: info_and_tag.png

I'm still going to look these over but I just noticed something worth noting.

Bug 1158853 removes the outline on the user text below the tile, but it doesn't look like we'll be uplifting it (unless I missed something?).

I attached a screenshot with the 'SUGGESTED' tag and user text outlined below it. Just seems kind of cluttered to me and I don't think that's how it was supposed to look in any mocks.

Any way we can get Bug 1158853 uplifted as well?

Comment 7

[Marina Samuel [:emtwo]]

Ah, nevermind. I was looking at the list from comment 4 not comment 5

Comment 8

[Marina Samuel [:emtwo]]

Looks good to me. I think all the bugs that need special handling for uplifting now have relevant patches included in them.

Comment 9

[Ed Lee :Mardak]

For 39 beta uplift, we'll need 6 more bugs that haven't been uplifted yet in addition to the ones uplifting from 41 to 40.

> pick a843ee9 Bug 1156549 - Allow ramp up time for campaigns with strict start/stop times [r=adw] - Maxim Zhilyaev
> pick a71bb38 Bug 1045766 - Add telemetry of users who can see new tab Tile suggestions [r=ttaubert] - Ed Lee
> pick fcfa8d9 Bug 1159571 - Use server provided frequency caps for daily and lifetime totals [r=adw] - Maxim Zhilyaev
> pick 45e0d5b Bug 1158853 - Replace history tile titles with base domains and include titles as part of tiles. r=adw - Marina Samuel
> pick f8957ee Bug 1158859 - Update newtab cog menu styling. r=adw - Marina Samuel
> pick 55657ef Bug 1138817 - Change [SPONSORED] tags to [SUGGESTED] and add [SUGGESTED] tags for suggested tiles. r=adw - Marina Samuel

Below are the 41 to 40 that also need to be in 39.

> pick 85b4893 Bug 1138818 - Part 1 - Onboarding UI without the tile images. r=adw - Marina Samuel
> pick e886493 Bug 1138818 - Part 2 - Add onboarding images. r=adw - Marina Samuel
> pick 3f7b612 Bug 1138818 - Part 3 - Change to localizable strings.  r=adw - Marina Samuel
> pick bdb9318 Bug 1160369 - Allow for sponsored suggested tiles [r=adw] - Ed Lee
> pick 4bafce9 Bug 1165386 - Always show SPONSORED for any type of sponsored tiles and SUGGESTED for other suggested [r=adw] - Ed Lee
> pick a661396 Bug 1165430 - Switch unlocalizable "Example Title" to "mozilla.org" [r=adw] - Ed Lee
> pick 33263c6 Bug 1139496 - Allow server provided explanation / ad group name to be displayed on Suggested Tiles [r=adw, f=bsmedberg] - Marina Samuel
> pick c60814f Bug 1166552 - browser_newtab_reportLinkAction.js fails on timeout [r=adw] - Ed Lee
> pick 99ab481 Bug 1159884 - Implement inadjacency with a hardcoded list of hashed sites [r=adw] - Maxim Zhilyaev
> pick 8c3e645 Bug 1165525 - Onboarding messaging poorly words how to turn off suggestions by implying blank is the only way [r=adw] - Ed Lee
> pick 0e1dc05 Bug 1159884 - Inadjacency documentation [f=bsmedberg] - Ed Lee
> pick 620576d Bug 1145428 - Suggested Tiles pins (becomes a user history tile after end time) [r=adw] - Maxim Zhilyaev
> pick f955a0a Bug 1167805 - Not enough padding for page title causing pin icon to overlap [r=adw] - Ed Lee
> pick f7251f5 Bug 1167243 - Intermittent browser_newtab_enhanced.js | Suggested for {'Web Education'|'Technology'} enthusiasts  [r=adw] - Marina Samuel

Of those some have special en-US beta only patches: bug 1138817, bug 1138818, bug 1158859, bug 1160369

Comment 10

[Ed Lee :Mardak]

Some had merge conflicts for 39 uplift that have beta specific patches: bug 1165386 (hardcoded SUGGESTED), bug 1139496 (enhanced2 vs enhanced), bug 1165525 (hardcoded intro strings)

Comment 11

[Ed Lee :Mardak]

Attachment: combined 11 patches for aurora (Mardak will land as separate commits)

Approval Request Comment
[Feature/regressing bug #]: This bug 1140185 (Sponsored Suggested Tiles)
[User impact if declined]: Some of sponsored suggested tiles is already in Aurora40 but not all the onboarding messaging logic was ready.
[Describe test coverage new/current, TreeHerder]: New logic changes have accompanying unit tests. Each of the 11 bugs have also been manually verified. These patches have been applied to mozilla-aurora and passing on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=232a324eea8f
[Risks and why]: Lowish given that the code has been on Nightly41 for a few days to few weeks with followup bugs triaged and fixed as necessary. These changes are mostly self-contained in the new tab / directory links feature without downstream dependencies. See diffstat at end.
[String/UUID change made/needed]: One patch for bug 1165525 has one string change that flod has f+ where the string is being changed without revving the string id. All other patches had their strings landed in Nightly40.

The 11 bugs for this combined patch:

$ git log gecko/aurora.. --format='format:> %h %s - %an' --reverse
> 9d490b4 Bug 1138818 - New tab user onboarding for sponsored suggested tiles. r=adw, a= - Marina Samuel
> 4ce0e93 Bug 1160369 - Allow for sponsored suggested tiles [r=adw, a=] - Ed Lee
> 14049b9 Bug 1165386 - Always show SPONSORED for any type of sponsored tiles and SUGGESTED for other suggested [r=adw, a=] - Ed Lee
> 36053cf Bug 1165430 - Switch unlocalizable "Example Title" to "mozilla.org" [r=adw, a=] - Ed Lee
> 53862c1 Bug 1139496 - Allow server provided explanation / ad group name to be displayed on Suggested Tiles [r=adw, f=bsmedberg, a=] - Marina Samuel
> 84b3a6f Bug 1166552 - browser_newtab_reportLinkAction.js fails on timeout [r=adw, a=] - Ed Lee
> 04aaa40 Bug 1159884 - Implement inadjacency with a hardcoded list of hashed sites [r=adw, f=bsmedberg, a=] - Maxim Zhilyaev
> 37be395 Bug 1165525 - Onboarding messaging poorly words how to turn off suggestions by implying blank is the only way [r=adw, f=flod, a=, l10n=] - Ed Lee
> 50ab7d5 Bug 1145428 - Suggested Tiles pins (becomes a user history tile after end time) [r=adw, a=] - Maxim Zhilyaev
> 684ca8a Bug 1167805 - Not enough padding for page title causing pin icon to overlap [r=adw, a=] - Ed Lee
> 38a104d Bug 1167243 - Intermittent browser_newtab_enhanced.js | Suggested for {'Web Education'|'Technology'} enthusiasts  [r=adw, a=] - Marina Samuel

$ diffstat sponsored.aurora.patch
 browser/app/profile/firefox.js                                      |    3 
 browser/base/content/newtab/grid.js                                 |    2 
 browser/base/content/newtab/intro.js                                |  234 
 browser/base/content/newtab/newTab.css                              |  265 
 browser/base/content/newtab/newTab.inadjacent.json                  | 3209 ++++++++++
 browser/base/content/newtab/newTab.xul                              |   29 
 browser/base/content/newtab/page.js                                 |    5 
 browser/base/content/newtab/sites.js                                |   66 
 browser/base/content/test/newtab/browser.ini                        |    1 
 browser/base/content/test/newtab/browser_newtab_bug1145428.js       |   88 
 browser/base/content/test/newtab/browser_newtab_enhanced.js         |   84 
 browser/base/content/test/newtab/browser_newtab_intro.js            |  108 
 browser/base/content/test/newtab/browser_newtab_reportLinkAction.js |   11 
 browser/base/jar.mn                                                 |    1 
 browser/docs/DirectoryLinksProvider.rst                             |   14 
 browser/locales/en-US/chrome/browser/newTab.properties              |    2 
 browser/modules/DirectoryLinksProvider.jsm                          |  158 
 browser/modules/test/xpcshell/test_DirectoryLinksProvider.js        |  226 
 browser/themes/linux/jar.mn                                         |    1 
 browser/themes/osx/jar.mn                                           |    1 
 browser/themes/shared/newtab/newTab.inc.css                         |   15 
 browser/themes/windows/jar.mn                                       |    1 
 testing/profiles/prefs_general.js                                   |    1 
 toolkit/modules/NewTabUtils.jsm                                     |   38 
 24 files changed, 4468 insertions(+), 95 deletions(-)

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Note that we have a particular marketing focus on dev edition 40 coming up in 6 days. We just learned today that the product team would like to disable updates to aurora on Thursday May 28. Are you comfortable with this going into aurora now, with there being no chance to ship fixes for anything that breaks before next Tuesday when we are re-enabling updates?

Comment 13

[Ed Lee :Mardak]

(In reply to Liz Henry (:lizzard) from comment #12)
> product team would like to disable updates to aurora on Thursday May 28
Clarified on IRC that this means there will be an update for tomorrow/Thursday and no more updates until next week Tuesday.

I believe we are good code-stability-wise to uplift the 11 patches from comment 11. This should give provide additional testing of tiles on Nightly and Aurora in preparation for uplifting patches to beta by 39.0 beta 2 go-to-build on June 1st.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8610942 [details] [diff] [review]
combined 11 patches for aurora (Mardak will land as separate commits)

Approved for uplift to aurora from discussion in the bug and on release-drivers.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

OK let's uplift this to aurora. From our conversation on IRC, I was trying to convey that you didn't have to fill a separate uplift request for each bug, or do a combined patch; just to fill out the form at least once.   What I was aiming for was for you to note your uplift request in a bug rather than in email, so that it would be understandable to many people like the entire release management team and the sheriffs. 

If there is a plan for QE to test and verify this feature on aurora, please link to it here or email it to release-drivers. Based on our agreed upon schedule, it needs to be verified by this Friday May 29 for release management to consider uplifting to 39 beta.  We will need to review the state of the feature after it is verified. 

Let me explain some context about the 39 release. 39 beta 1 is ready, but has not been able to release yet on the beta channel due to 38.0.5 holding it up. This means the feature can't make Beta 1. Beta 2 will go to build on Monday June 1.  This means that we will have about 3 and a half weeks on beta and at best, only 5 beta releases. 39 will be in RC during the Whistler all hands work week. 

Are there patches that still haven't landed on 41 that will need uplift?   For example,  Bug 1105376 - Support negative adjacency of tiles to not show a tile in the context of a visible site and Bug 1158230 - Improve technical guarantees for Suggested Tiles launch. How will that affect testing?

Comment 16

[Ed Lee :Mardak]

Removing bug 1136977 and bug 1158230 as the remaining work is server changes that don't affect Firefox code or functionality.

All the bugs with qe-verify+ have been verified on Aurora41:
https://bugzilla.mozilla.org/buglist.cgi?o1=equals&o2=anywords&v1=1140185&v2=verified&f1=blocked&o3=nowords&v3=fixed%20verified&query_format=advanced&f3=cf_status_firefox39&f2=cf_status_firefox40

There's a couple test fixes that were marked qe-verify-:
https://bugzilla.mozilla.org/buglist.cgi?o1=equals&o2=anywords&v1=1140185&v2=fixed&f1=blocked&o3=nowords&v3=fixed%20verified&query_format=advanced&f3=cf_status_firefox39&f2=cf_status_firefox40

Cornel Ionce verified these bugs early this morning per https://wiki.mozilla.org/QA/Suggested_Tiles/Test_Plan

Comment 17

[Ed Lee :Mardak]

To be clear, all 17 bugs for Beta39 uplift have been verified on Nightly41+Aurora40, so we should be good from engineering and QE for the Monday June 1 go-to-build of Beta39 b2.

Comment 18

[Ed Lee :Mardak]

Attachment: combined 17 patches for beta (Mardak will land as separate commits)

Approval Request Comment
[Feature/regressing bug #]: This bug 1140185 (Sponsored Suggested Tiles)
[User impact if declined]: Affiliate suggested tiles bug 1120311 is already part of 39, but it lacks the full onboarding experience added by the set of features for this bug as well as the additional controls/logic that limits when suggestions are made.
[Describe test coverage new/current, TreeHerder]: As per comment 11 for aurora uplift, there's the existing test coverage. For the 6 additional bugs uplifting from aurora to beta, those have unit tests where appropriate (no test for styling changes or telemetry probe). These bugs have been verified on Nightly41 and Aurora40. https://treeherder.mozilla.org/#/jobs?repo=try&revision=f01457ed15dc
[Risks and why]: Similar to comment 11 for Nightly41 to Aurora40 uplift. The 6 additional patches have been on Aurora40 for a while now, so uplifting to Beta39 should be low risk. There are some differences to specially handle en-US-only changes that only exists for 39, but we have unit tests covering these differences.
[String/UUID change made/needed]: None. en-US-only strings are being uplifted as javascript code without touching locales/.dtd|properties files with appropriate logic to show those new strings for en-US and existing strings for others.

$ git log gecko/beta.. --format='format:> %h %s - %an' --reverse
> dcb82e9 Bug 1156549 - Allow ramp up time for campaigns with strict start/stop times [r=adw, f=bsmedberg, a=] - Maxim Zhilyaev
> d6fe437 Bug 1045766 - Add telemetry of users who can see new tab Tile suggestions [r=ttaubert, f=bsmedberg, a=] - Ed Lee
> e3f6cdd Bug 1159571 - Use server provided frequency caps for daily and lifetime totals [r=adw, f=bsmedberg, a=] - Maxim Zhilyaev
> 14889a3 Bug 1158853 - Replace history tile titles with base domains and include titles as part of tiles. r=adw, a= - Marina Samuel
> b62bc3e Bug 1158859 - Update newtab cog menu styling. r=adw, a= - Marina Samuel
> 01d4d61 Bug 1138817 - Change [SPONSORED] tags to [SUGGESTED] and add [SUGGESTED] tags for suggested tiles. r=adw, r=Mardak, a= - Marina Samuel
> 3a9e4cf Bug 1138818 - New tab user onboarding for sponsored suggested tiles. r=adw, a= - Marina Samuel
> 7547ea1 Bug 1160369 - Allow for sponsored suggested tiles [r=adw, r=Mardak, a=] - Marina Samuel
> 8b0072e Bug 1165386 - Always show SPONSORED for any type of sponsored tiles and SUGGESTED for other suggested [r=adw, a=] - Ed Lee
> ffa791a Bug 1165430 - Switch unlocalizable "Example Title" to "mozilla.org" [r=adw, a=] - Ed Lee
> b88a03b Bug 1139496 - Allow server provided explanation / ad group name to be displayed on Suggested Tiles [r=adw, f=bsmedberg, a=] - Marina Samuel
> 8488a6e Bug 1166552 - browser_newtab_reportLinkAction.js fails on timeout [r=adw, a=] - Ed Lee
> 10287b8 Bug 1159884 - Implement inadjacency with a hardcoded list of hashed sites [r=adw, f=bsmedberg, a=] - Maxim Zhilyaev
> d82ba85 Bug 1165525 - Onboarding messaging poorly words how to turn off suggestions by implying blank is the only way [r=adw, a=] - Ed Lee
> 50412e8 Bug 1145428 - Suggested Tiles pins (becomes a user history tile after end time) [r=adw, a=] - Maxim Zhilyaev
> b195a6b Bug 1167805 - Not enough padding for page title causing pin icon to overlap [r=adw, a=] - Ed Lee
> 95db8d8 Bug 1167243 - Intermittent browser_newtab_enhanced.js | Suggested for {'Web Education'|'Technology'} enthusiasts  [r=adw, a=] - Marina Samuel

$ diffstat sponsored.beta.patch 
 browser/app/profile/firefox.js                                      |    3 
 browser/base/content/newtab/customize.js                            |   70 
 browser/base/content/newtab/intro.js                                |  267 
 browser/base/content/newtab/newTab.css                              |  403 +
 browser/base/content/newtab/newTab.inadjacent.json                  | 3209 ++++++++++
 browser/base/content/newtab/newTab.xul                              |   53 
 browser/base/content/newtab/page.js                                 |    5 
 browser/base/content/newtab/sites.js                                |   95 
 browser/base/content/test/newtab/browser.ini                        |    1 
 browser/base/content/test/newtab/browser_newtab_bug1145428.js       |   88 
 browser/base/content/test/newtab/browser_newtab_enhanced.js         |   98 
 browser/base/content/test/newtab/browser_newtab_intro.js            |  124 
 browser/base/content/test/newtab/browser_newtab_reportLinkAction.js |   11 
 browser/base/content/test/newtab/head.js                            |    2 
 browser/base/jar.mn                                                 |    1 
 browser/docs/DirectoryLinksProvider.rst                             |   22 
 browser/modules/DirectoryLinksProvider.jsm                          |  550 +
 browser/modules/test/xpcshell/test_DirectoryLinksProvider.js        |  679 ++
 browser/themes/linux/jar.mn                                         |    1 
 browser/themes/osx/jar.mn                                           |    1 
 browser/themes/shared/newtab/newTab.inc.css                         |   41 
 browser/themes/windows/jar.mn                                       |    1 
 testing/profiles/prefs_general.js                                   |    1 
 toolkit/components/telemetry/Histograms.json                        |    5 
 toolkit/modules/NewTabUtils.jsm                                     |   59 
 toolkit/themes/linux/global/jar.mn                                  |    3 
 toolkit/themes/osx/global/jar.mn                                    |    3 
 toolkit/themes/shared/menu-check-active.svg                         |    6 
 toolkit/themes/shared/menu-check-black.svg                          |    6 
 toolkit/themes/shared/menu-check-hover.svg                          |    6 
 toolkit/themes/windows/global/jar.mn                                |    3 
 31 files changed, 5647 insertions(+), 170 deletions(-)

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8612422 [details] [diff] [review]
combined 17 patches for beta (Mardak will land as separate commits)

As discussed in email and with Darren we are going to keep these changes on 40 to release with 40 and not uplift them to beta.

Comment 20

[Ed Lee :Mardak]

emtwo, can you create patches for aurora uplift for these bugs:

https://bugzilla.mozilla.org/buglist.cgi?f1=blocked&o1=equals&v1=1140185&f2=cf_status_firefox40&o2=nowords&v2=fixed%20verified&j3=OR&f3=OP&f4=cf_status_firefox41&o4=anywords&v4=fixed%20verified

Currently that's 4 bugs:

Bug 1158858 Move 'suggested' and 'sponsored' labels to the top left of the tile
Bug 1164594 New Tab shouldn't use text-transform: uppercase
Bug 1165594 New Tab controls looks broken with RTL locales
Bug 1172987 Incorrect padding for various locales

Comment 21

[Marina Samuel [:emtwo]]

I've updated the bugs respectively