Open Bug 1140185 Opened 5 years ago Updated 9 months ago

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

Categories

(Content Services Graveyard :: Tiles, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: Mardak, Unassigned)

References

Details

(Keywords: meta, Whiteboard: [story])

Attachments

(3 files)

Tracking bug for suggested tiles functionality that won't be part of the initial feature.
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [epic]
Depends on: 1120311
Depends on: 1138817
Depends on: 1136461
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.
Summary: Followup suggested tiles work → Sponsored suggested tiles
Whiteboard: [epic] → [story]
Depends on: 1121549
Summary: Sponsored suggested tiles → Suggested Tiles: Sponsored Release (FX 39)
Depends on: 1139496
Depends on: 1138818
Summary: Suggested Tiles: Sponsored Release (FX 39) → Suggested Tiles: v1.0 Sponsored Release (FX 39)
Blocks: 1145418
Goes to Aurora: 2015-03-30
Goes to Beta:   2015-05-11
Release Date:   2015-06-30
Depends on: 1145428
Depends on: 1150228
Depends on: 1151876
Depends on: 1155443
No longer depends on: 1155443
Depends on: 1156537
Depends on: 1156542
Depends on: 1105376
Depends on: 1128560
Depends on: 1045766
Depends on: 1136977
Depends on: 1158230
Depends on: 1159571
Depends on: 1145303
No longer depends on: 1136461
Depends on: 1158859
Depends on: 1158858
Depends on: 1158856
No longer depends on: 1121549
Depends on: 1158853
Depends on: 1160369
Depends on: 1160372
Depends on: 1156549
Depends on: 1159884
Depends on: 1161412
No longer depends on: 1161412
No longer depends on: 1160372
No longer depends on: 1128560
Depends on: 1165386
No longer depends on: 1145303
No longer depends on: 1158856
No longer depends on: 1158858
Depends on: 1165525
Depends on: 1167243
Depends on: 1167805
Depends on: 1166552
Depends on: 1165430
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
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).
Flags: needinfo?(mzhilyaev)
Flags: needinfo?(msamuel)
Attached image 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?
Flags: needinfo?(edilee)
Ah, nevermind. I was looking at the list from comment 4 not comment 5
Flags: needinfo?(edilee)
Looks good to me. I think all the bugs that need special handling for uplifting now have relevant patches included in them.
Flags: needinfo?(msamuel)
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
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)
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(-)
Flags: needinfo?(mzhilyaev)
Attachment #8610942 - Flags: approval-mozilla-aurora?
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?
Flags: needinfo?(kghim)
Flags: needinfo?(edilee)
(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.
Flags: needinfo?(edilee)
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.
Attachment #8610942 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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?
Flags: needinfo?(kghim)
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.
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(-)
Attachment #8612422 - Flags: approval-mozilla-beta?
Depends on: 1169366
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.
Attachment #8612422 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Summary: Suggested Tiles: v1.0 Sponsored Release (FX 39) → Suggested Tiles: v1.0 Sponsored Release (FX 40)
Depends on: 1168898
Depends on: 1167616
Depends on: 1165394
Depends on: 1165594
Depends on: 1158858
Depends on: 1164594
Depends on: 1171116
Depends on: 1171112
Depends on: 1172987
No longer depends on: 1165394
Depends on: 1172721
Depends on: 1168521
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
Flags: needinfo?(msamuel)
I've updated the bugs respectively
Flags: needinfo?(msamuel)
Depends on: 1176985
Depends on: 1178180
No longer depends on: 1167616
You need to log in before you can comment on or make changes to this bug.