We'll want to add a paragraph explaining suggested tiles and maybe(?) retrigger the first-run experience to have the "what's this page" dialog appear. kghim, who should be needinfo? to provide the intro text?

Ed, I think we should approach the Suggested Tiles intro this way: 1) Figure out constraints in showing introduction overlay on new tab (athornburgh) 2) Figure out complexity in implementation (msamuel) 3) Determine appropriate description text/design based on 1 & 2 (kghim/pfinch/athornburgh) First step is on athornburgh.

I have a 1-on-1 with Madhava tomorrow (Wed), and was going to talk with him about it then. I'll report back immediately after in this bug with the outcome. He's already seen this in both design and prototype form, so we should all be on the same page from a UX/UI perspective. -A

For new user and existing users. items: - Suggested Tiles intro - Existing Tiles control functionality - Disclaimer overlay - New tab options

This will most likely replace the "what is this page" link and related popup. Not sure if we trigger the onboarding flow again if the user clicks "what is this page" as a menu item where somewhere in the flow, there's a link to the firefox/tiles page?

Copying over dcrobot's bug 1138817 comment 10: Attached are ALL the comps for Tiles on New Tab for Firefox 39, including the on-boarding experience. All comments from Kevin have been incorporated. If anyone else has feedback, please respond by EOD today (3-20-2015).

> Created attachment 8580819 [details] > Attached are ALL the comps for Tiles on New Tab for Firefox 39, including the > on-boarding experience. A few things to note: - the size of the user's screen isn't guaranteed, so if we naively center the messaging, it could cover up the thing we're highlighting - we can't guarantee a suggested or sponsored tile for every locale to be available during onboarding, so it might make sense to ship a dummy suggested/sponsored tile (whimsycorn?) to convey the concepts - do we need to be more explicit about some of the items that are currently in the "what is this page?" messaging? e.g., data analytics notice

Ed... - Let's talk more about this to determine either a technical or design solution. What I've proposed was intended to be independent of browser window size. - Why can't we just show all users w/o a suitable match a Mozilla tile? That way everyone get's an actual ad... but some will see one from Mozilla, and others from a partner. No unicorns ;) - The on-boarding UI includes links to all of those resources. If we need to say something specific within this experience, I need to know exactly what the words should say and any rules around how they should be displayed. From there, I can figure something out. Will need Geoff to weigh in on this.

Considering the scope of this bug, we'll need to release into Nightly 40, then uplift to Beta 39. Merge date is 2015-05-11.

Additionally, when a new user has seen the onboarding, do not display it again during history tile (existing user phase).

(In reply to Kevin Ghim from comment #8) > Considering the scope of this bug, we'll need to release into Nightly 40, > then uplift to Beta 39. Merge date is 2015-05-11. Whiteboard: .? → .002 If this is targeting Nightly 40, then the whiteboard priority should be nowhere close to .002

> - the size of the user's screen isn't guaranteed, so if we naively center > the messaging, it could cover up the thing we're highlighting Perhaps we can not highlight tiles if the screen is smaller than a certain size?

> - Why can't we just show all users w/o a suitable match a Mozilla tile? That > way everyone get's an actual ad... but some will see one from Mozilla, and > others from a partner. No unicorns ;) If a user is shown a Mozilla affiliate tile, it won't have a 'suggested' tag correct? Won't this defeat the purpose of telling users 'Firefox only suggests sites that match your interests' while showing them what a suggested tile looks like? Also, we can't guarantee that certain tiles will be in the location where we want to highlight them. Can we choose certain tiles and place them on top of the overlay just for demonstration purposes instead of 'highlighting' the tiles underneath?

My understanding was that the Mozilla suggested tiles would be released on Fx 38... and for Fx 39 we're introducing suggested tiles from partners. If this is still true, then the user should NOT see the on-boarding UNTIL an appropriate partner tile is available. Furthermore, according to the revised comps, ALL suggested tiles are labeled as such, regardless of whether it's a Mozilla or partner tile. For the minority of users who have very small screens, I think we should talk about this as a team... But personally, I'm ok if we don't show the tile examples since the text in the layover window is sufficient. The one exception is the New Tab Controls menu, as this is literally the most important thing we need to show (both legally, and in terms of user control). -A

Why did you remove the blocking bug 1145303?

This patch includes the majority of the on-boarding UI based on https://bug1150228.bugzilla.mozilla.org/attachment.cgi?id=8592387 (pages 11-20) It's only based on the small screen UI seen in the last few pages (the larger screen UI is a separate bug) I'll add the images for each page of the onboarding in a 'part 2' patch; I want to get some of the bugs blocking bug 1150228 done first so I can reuse the styling.

Removed some unused code & adjusted a comment.

Here's a build to test it out: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2dd7ffa0830d I'll also be updating the intro.js test in a bit.

Tests added

Before removing the existing panel UI, we'll want to land it in a way that Firefox could show the old panel or the new onboarding. This is so that we can uplift to Aurora 39 an onboarding experience that is en-US only. For that uplift, we'll also need to land strings in a way that doesn't break string freeze similar to what we needed to do for bug 1141617. I'm thinking something along the lines of.. intro.js: if DirectoryLinksProvider.locale == en-US do onboarding if necessary else do old panel if necessary And then we'll have a followup bug that lands in Nightly 40 that removes the check and always does onboarding check and panel code can be removed.

Comment on attachment 8598762 [details] [diff] [review] Part 1: v3: Onboarding UI without the tile images >+newtab.intro.delete=delete Per bug 1150228 comment 24/25, we should be consistent with the existing "remove" label instead of "delete".

Hardcoded strings & onboarding for en-US. Old intro is shown for non-en-US.

Comment on attachment 8598762 [details] [diff] [review] Part 1: v3: Onboarding UI without the tile images ># HG changeset patch ># Parent f214df6ac75f0202e40733be62c5b169d7a40b7f ># User Marina Samuel <msamuel@mozilla.com> > >diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js >--- a/browser/app/profile/firefox.js >+++ b/browser/app/profile/firefox.js >@@ -1615,16 +1615,19 @@ pref("browser.panorama.animate_zoom", tr > // Defines the url to be used for new tabs. > pref("browser.newtab.url", "about:newtab"); > // Activates preloading of the new tab url. > pref("browser.newtab.preload", true); > > // Remembers if the about:newtab intro has been shown > pref("browser.newtabpage.introShown", false); > >+// Remembers if the about:newtab update intro has been shown >+pref("browser.newtabpage.updateIntroShown", false); >+ > // Toggles the content of 'about:newtab'. Shows the grid when enabled. > pref("browser.newtabpage.enabled", true); > > // number of rows of newtab grid > pref("browser.newtabpage.rows", 3); > > // number of columns of newtab grid > pref("browser.newtabpage.columns", 5); >diff --git a/browser/base/content/newtab/intro.js b/browser/base/content/newtab/intro.js >--- a/browser/base/content/newtab/intro.js >+++ b/browser/base/content/newtab/intro.js >@@ -1,57 +1,165 @@ > #ifdef 0 > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this file, > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > #endif > > const PREF_INTRO_SHOWN = "browser.newtabpage.introShown"; >+const PREF_UPDATE_INTRO_SHOWN = "browser.newtabpage.updateIntroShown"; >+ >+// These consts indicate the type of intro/onboarding we show. >+const WELCOME = "welcome"; >+const UPDATE = "update"; >+ >+// The maximum paragraph ID listed for 'newtab.intro.paragraph' >+// strings in newTab.properties >+const MAX_PARAGRAPH_ID = 9; >+ >+const NUM_INTRO_PAGES = 3; > > let gIntro = { > _nodeIDSuffixes: [ >- "panel", >- "what", >+ "mask", >+ "modal", >+ "numerical-progress", >+ "text", >+ "buttons", >+ "header", >+ "footer" > ], > >+ _buttonStrings: { >+ "continue": newTabString("intro.continue"), >+ "skip": newTabString("intro.skip"), >+ "next": newTabString("intro.next"), >+ "back": newTabString("intro.back"), >+ "gotit": newTabString("intro.gotit") >+ }, >+ >+ /** >+ * The paragraphs & buttons to show on each page in the intros. >+ */ >+ _introPages: { >+ "welcome": [[0,1],[2,3],[4,5]], >+ "update": [[6,5],[4,3],[0,1]], >+ "buttons": [["skip", "continue"],["back", "next"],["back", "gotit"]] >+ }, >+ >+ _paragraphs: [], >+ > _nodes: {}, > > init: function() { > for (let idSuffix of this._nodeIDSuffixes) { > this._nodes[idSuffix] = document.getElementById("newtab-intro-" + idSuffix); > } >+ }, > >- this._nodes.panel.addEventListener("popupshowing", e => this._setUpPanel()); >- this._nodes.panel.addEventListener("popuphidden", e => this._hidePanel()); >- this._nodes.what.addEventListener("click", e => this.showPanel()); >+ _goToPage: function(pageNum) { >+ this._currPage = pageNum; >+ >+ this._nodes["numerical-progress"].innerHTML = `${this._bold(pageNum + 1)}/ ${NUM_INTRO_PAGES}`; >+ this._nodes["numerical-progress"].setAttribute("page", pageNum); >+ >+ // Set the paragraphs >+ let paragraphNodes = this._nodes.text.getElementsByTagName("p"); >+ let paragraphIDs = this._introPages[this._onboardingType][pageNum]; >+ paragraphIDs.forEach((arg, index) => { >+ paragraphNodes[index].innerHTML = this._paragraphs[arg]; >+ }); >+ >+ // Set the buttons >+ let buttonNodes = this._nodes.buttons.getElementsByTagName("input"); >+ let buttonIDs = this._introPages.buttons[pageNum]; >+ buttonIDs.forEach((arg, index) => { >+ buttonNodes[index].setAttribute("value", this._buttonStrings[arg]); >+ }); >+ }, >+ >+ _bold: function(str) { >+ return `<strong> ${str} </strong>` >+ }, >+ >+ _link: function(url, text) { >+ return '<a href="' + url + '" target="_blank">' + text + "</a>" >+ }, >+ >+ _exitIntro: function() { >+ this._nodes.mask.style.opacity = 0; >+ this._nodes.mask.addEventListener("transitionend", () => { >+ this._nodes.mask.style.display = "none"; >+ }); >+ }, >+ >+ _back: function() { >+ if (this._currPage == 0) { >+ // We're on the first page so 'back' means exit. >+ this._exitIntro(); >+ return; >+ } >+ this._goToPage(this._currPage - 1); >+ }, >+ >+ _next: function() { >+ if (this._currPage == (NUM_INTRO_PAGES - 1)) { >+ // We're on the last page so 'next' means exit. >+ this._exitIntro(); >+ return; >+ } >+ this._goToPage(this._currPage + 1); >+ }, >+ >+ _generateParagraphs: function() { >+ let customizeIcon = '<input type="button" class="newtab-control newtab-customize"/>'; >+ >+ let substringMappings = { >+ "2": [this._link(TILES_PRIVACY_LINK, newTabString("privacy.link"))], >+ "4": [customizeIcon, this._bold(newTabString("intro.controls"))], >+ "6": [this._bold(newTabString("intro.delete")), this._bold(newTabString("intro.pin"))], >+ "7": [this._link(TILES_INTRO_LINK, newTabString("learn.link"))], >+ "8": [this._link(TILES_INTRO_LINK, newTabString("learn.link"))] >+ } >+ >+ for (let i = 1; i <= MAX_PARAGRAPH_ID; i++) { >+ try { >+ this._paragraphs.push(newTabString("intro.paragraph" + i, substringMappings[i])) >+ } catch (ex) { >+ // Paragraph with this ID doesn't exist so continue >+ } >+ } > }, > > showIfNecessary: function() { > if (!Services.prefs.getBoolPref(PREF_INTRO_SHOWN)) { >- Services.prefs.setBoolPref(PREF_INTRO_SHOWN, true); >+ this._onboardingType = WELCOME; >+ this.showPanel(); >+ } else if (!Services.prefs.getBoolPref(PREF_UPDATE_INTRO_SHOWN)) { >+ this._onboardingType = UPDATE; > this.showPanel(); > } >+ Services.prefs.setBoolPref(PREF_INTRO_SHOWN, true); >+ Services.prefs.setBoolPref(PREF_UPDATE_INTRO_SHOWN, true); > }, > > showPanel: function() { >- // Point the panel at the 'what' link >- this._nodes.panel.hidden = false; >- this._nodes.panel.openPopup(this._nodes.what); >+ this._nodes.mask.style.display = "block"; >+ this._nodes.mask.style.opacity = 1; >+ >+ if (!this._paragraphs.length) { >+ // It's our first time showing the panel. Do some initial setup >+ this._generateParagraphs(); >+ } >+ this._goToPage(0); >+ >+ // Header text >+ this._nodes.header.innerHTML = newTabString("intro.header." + this._onboardingType); >+ >+ // Footer links >+ let footerLinkNodes = this._nodes.footer.getElementsByTagName("li"); >+ [this._link(TILES_INTRO_LINK, newTabString("learn.link2")), >+ this._link(TILES_PRIVACY_LINK, newTabString("privacy.link2")), >+ ].forEach((arg, index) => { >+ footerLinkNodes[index].innerHTML = arg; >+ }); > }, >- >- _setUpPanel: function() { >- // Build the panel if necessary >- if (this._nodes.panel.childNodes.length == 1) { >- ['<a href="' + TILES_INTRO_LINK + '">' + newTabString("learn.link") + "</a>", >- '<a href="' + TILES_PRIVACY_LINK + '">' + newTabString("privacy.link") + "</a>", >- '<input type="button" class="newtab-customize"/>', >- ].forEach((arg, index) => { >- let paragraph = document.createElementNS(HTML_NAMESPACE, "p"); >- this._nodes.panel.appendChild(paragraph); >- paragraph.innerHTML = newTabString("intro.paragraph" + (index + 1), [arg]); >- }); >- } >- }, >- >- _hidePanel: function() { >- this._nodes.panel.hidden = true; >- } > }; >diff --git a/browser/base/content/newtab/newTab.css b/browser/base/content/newtab/newTab.css >--- a/browser/base/content/newtab/newTab.css >+++ b/browser/base/content/newtab/newTab.css >@@ -533,8 +533,215 @@ input[type=button] { > background-image: url("chrome://global/skin/menu/shared-menu-check@2x.png"); > } > } > > .searchSuggestionTable { > font: message-box; > font-size: 16px; > } >+ >+/** >+ * Onboarding styling >+ */ >+ >+ #newtab-intro-mask { >+ position: fixed; >+ top: 0; >+ left: 0; >+ width: 100%; >+ height: 100%; >+ background: #424F5A; >+ z-index:100; >+ background-color: rgba(66,79,90,0.95);; >+ transition: opacity .5s linear; >+ overflow: auto; >+ display: none; >+} >+ >+#newtab-intro-modal { >+ font-family: "Helvetica"; >+ width: 700px; >+ height: 500px; >+ position: fixed; >+ left: 0; >+ right: 0; >+ top: 0; >+ bottom: 0; >+ margin: auto; >+ background: linear-gradient(#FFFFFF, #F9F9F9); >+ box-shadow: 0px 2px 4px #C1C1C1; >+ border-radius: 8px 8px 0px 0px; >+} >+ >+#newtab-intro-header { >+ font-size: 28px; >+ color: #737980; >+ text-align: center; >+ top: 50px; >+ position: relative; >+ border-bottom: 2px solid #E0DFE0; >+ padding-bottom: 10px; >+ width: 600px; >+ display: block; >+ margin: 0px auto; >+ font-weight: 100; >+} >+ >+#newtab-intro-footer { >+ width: 100%; >+ height: 55px; >+ margin: 0px auto; >+ display: block; >+ position: absolute; >+ bottom: 0px; >+ background-color: white; >+ box-shadow: 0px -1px 4px #EBEBEB; >+ text-align: center; >+ vertical-align: middle; >+ line-height: 55px; >+} >+ >+#newtab-intro-footer > ul { >+ list-style-type: none; >+ margin: 0px; >+ padding: 0px; >+} >+ >+#newtab-intro-footer > ul > li { >+ display: inline; >+ padding-left: 10px; >+ padding-right: 10px; >+} >+ >+#newtab-intro-footer > ul > li > a { >+ text-decoration: none; >+ color: #4A90E2; >+} >+ >+#newtab-intro-footer > ul > li > a:visited { >+ color: #171F26; >+} >+ >+#newtab-intro-footer > ul > :first-child { >+ border-right: solid 1px #C1C1C1; >+} >+ >+#newtab-intro-body { >+ height: 300px; >+ position: relative; >+ display: block; >+ top: 50px; >+ margin: 25px 50px 30px; >+} >+ >+#newtab-intro-content > * { >+ display: inline-block; >+} >+ >+#newtab-intro-content { >+ height: 210px; >+ position: relative; >+} >+ >+#newtab-intro-buttons { >+ height: 90px; >+ text-align: center; >+ vertical-align: middle; >+ line-height: 90px; >+} >+ >+#newtab-intro-tile { >+ width: 290px; >+ height: 100%; >+} >+ >+#newtab-intro-text { >+ height: 100%; >+ width: 270px; >+ right: 0px; >+ position: absolute; >+ font-size: 14px; >+ line-height: 20px; >+} >+ >+#newtab-intro-text > p { >+ margin: 0 0 1em 0; >+} >+ >+#newtab-intro-text .newtab-control { >+ background-size: 18px auto; >+ height: 18px; >+ width: 18px; >+ vertical-align: middle; >+ opacity: 1; >+ position: inherit; >+} >+ >+#newtab-intro-buttons > input { >+ width: 150px; >+ height: 50px; >+ margin: 0px 5px; >+ vertical-align: bottom; >+ border-radius: 2px; >+ border: solid 1px #2C72E2; >+ background-color: #FFFFFF; >+ color: #4A90E2; >+} >+ >+#newtab-intro-buttons > input[default] { >+ background-color: #4A90E2; >+ color: #FFFFFF; >+} >+ >+#newtab-intro-buttons > input:hover { >+ background-color: #2C72E2; >+ color: #FFFFFF; >+} >+ >+#newtab-intro-progress { >+ position: absolute; >+ width: 100%; >+} >+ >+#newtab-intro-numerical-progress { >+ text-align: center; >+ top: 15px; >+ position: relative; >+ font-size: 12px; >+ color: #424F5A; >+} >+ >+#newtab-intro-graphical-progress { >+ text-align: left; >+ border-radius: 1.5px; >+ overflow: hidden; >+ position: relative; >+ margin: 10px auto 0px; >+ height: 3px; >+ top: 8px; >+ width: 35px; >+ opacity: 0.25; >+ background-color: #DCDCDC; >+} >+ >+#indicator { >+ position: absolute; >+ top: 0px; >+ left: 0px; >+ display: inline-block; >+ width: 0%; >+ height: 4px; >+ background: none repeat scroll 0% 0% #FF9500; >+ transition: width 0.3s ease-in-out 0s; >+} >+ >+#newtab-intro-numerical-progress[page="0"] + #newtab-intro-graphical-progress > #indicator { >+ width: 33%; >+} >+ >+#newtab-intro-numerical-progress[page="1"] + #newtab-intro-graphical-progress > #indicator { >+ width: 66%; >+} >+ >+#newtab-intro-numerical-progress[page="2"] + #newtab-intro-graphical-progress > #indicator { >+ width: 100%; >+} >diff --git a/browser/base/content/newtab/newTab.xul b/browser/base/content/newtab/newTab.xul >--- a/browser/base/content/newtab/newTab.xul >+++ b/browser/base/content/newtab/newTab.xul >@@ -42,16 +42,45 @@ > <xul:hbox id="newtab-customize-classic" class="newtab-customize-panel-item"> > <xul:label>&newtab.customize.topsites;</xul:label> > </xul:hbox> > <xul:hbox id="newtab-customize-blank" class="newtab-customize-panel-item"> > <xul:label>&newtab.customize.blank2;</xul:label> > </xul:hbox> > </xul:panel> > >+ <div id="newtab-intro-mask"> >+ <div id="newtab-intro-modal"> >+ <div id="newtab-intro-progress"> >+ <div id="newtab-intro-numerical-progress"/> >+ <div id="newtab-intro-graphical-progress"> >+ <span id="indicator"/> >+ </div> >+ </div> >+ <div id="newtab-intro-header"/> >+ <div id="newtab-intro-body"> >+ <div id="newtab-intro-content"> >+ <div id="newtab-intro-tile"/> >+ <div id="newtab-intro-text"> >+ <p/><p/> >+ </div> >+ </div> >+ <div id="newtab-intro-buttons"> >+ <input type="button" onclick="gIntro._back()"/> >+ <input type="button" default="true" onclick="gIntro._next()"/> >+ </div> >+ </div> >+ <div id="newtab-intro-footer"> >+ <ul> >+ <li/><li/> >+ </ul> >+ </div> >+ </div> >+ </div> >+ > <div id="newtab-scrollbox"> > > <div id="newtab-vertical-margin"> > > <div id="newtab-margin-top"/> > > <div id="newtab-margin-undo-container"> > <div id="newtab-undo-container" undo-disabled="true"> >diff --git a/browser/base/content/test/newtab/browser_newtab_intro.js b/browser/base/content/test/newtab/browser_newtab_intro.js >--- a/browser/base/content/test/newtab/browser_newtab_intro.js >+++ b/browser/base/content/test/newtab/browser_newtab_intro.js >@@ -1,51 +1,94 @@ > /* Any copyright is dedicated to the Public Domain. > http://creativecommons.org/publicdomain/zero/1.0/ */ > > const INTRO_PREF = "browser.newtabpage.introShown"; >+const UPDATE_INTRO_PREF = "browser.newtabpage.updateIntroShown"; > const PRELOAD_PREF = "browser.newtab.preload"; > > function runTests() { > let origIntro = Services.prefs.getBoolPref(INTRO_PREF); >+ let origUpdateIntro = Services.prefs.getBoolPref(UPDATE_INTRO_PREF); > let origPreload = Services.prefs.getBoolPref(PRELOAD_PREF); > registerCleanupFunction(_ => { > Services.prefs.setBoolPref(INTRO_PREF, origIntro); >+ Services.prefs.setBoolPref(INTRO_PREF, origUpdateIntro); > Services.prefs.setBoolPref(PRELOAD_PREF, origPreload); > }); > > // Test with preload false > Services.prefs.setBoolPref(INTRO_PREF, false); >+ Services.prefs.setBoolPref(UPDATE_INTRO_PREF, false); > Services.prefs.setBoolPref(PRELOAD_PREF, false); > >- let panel; >- function maybeWaitForPanel() { >- // If already open, no need to wait >- if (panel.state == "open") { >- executeSoon(TestRunner.next); >- return; >- } >- >- // We're expecting the panel to open, so wait for it >- panel.addEventListener("popupshown", TestRunner.next); >- isnot(panel.state, "open", "intro panel can be slow to show"); >- } >+ let intro; >+ yield addNewTabPageTab(); >+ intro = getContentDocument().getElementById("newtab-intro-mask"); >+ is(intro.style.opacity, 1, "intro automatically shown on first opening"); >+ is(getContentDocument().getElementById("newtab-intro-header").innerHTML, >+ "Welcome to New Tab on Firefox!", "we show the first-run intro."); >+ is(Services.prefs.getBoolPref(INTRO_PREF), true, "newtab remembers that the intro was shown"); >+ is(Services.prefs.getBoolPref(UPDATE_INTRO_PREF), true, "newtab avoids showing update if intro was shown"); > > yield addNewTabPageTab(); >- panel = getContentDocument().getElementById("newtab-intro-panel"); >- yield maybeWaitForPanel(); >- is(panel.state, "open", "intro automatically shown on first opening"); >- is(Services.prefs.getBoolPref(INTRO_PREF), true, "newtab remembers that the intro was shown"); >- >- yield addNewTabPageTab(); >- panel = getContentDocument().getElementById("newtab-intro-panel"); >- is(panel.state, "closed", "intro not shown on second opening"); >+ intro = getContentDocument().getElementById("newtab-intro-mask"); >+ is(intro.style.opacity, 0, "intro not shown on second opening"); > > // Test with preload true > Services.prefs.setBoolPref(INTRO_PREF, false); > Services.prefs.setBoolPref(PRELOAD_PREF, true); > > yield addNewTabPageTab(); >- panel = getContentDocument().getElementById("newtab-intro-panel"); >- yield maybeWaitForPanel(); >- is(panel.state, "open", "intro automatically shown on preloaded opening"); >+ intro = getContentDocument().getElementById("newtab-intro-mask"); >+ is(intro.style.opacity, 1, "intro automatically shown on preloaded opening"); >+ is(getContentDocument().getElementById("newtab-intro-header").innerHTML, >+ "Welcome to New Tab on Firefox!", "we show the first-run intro."); > is(Services.prefs.getBoolPref(INTRO_PREF), true, "newtab remembers that the intro was shown"); >+ is(Services.prefs.getBoolPref(UPDATE_INTRO_PREF), true, "newtab avoids showing update if intro was shown"); >+ >+ // Test with first run true but update false >+ Services.prefs.setBoolPref(UPDATE_INTRO_PREF, false); >+ >+ yield addNewTabPageTab(); >+ intro = getContentDocument().getElementById("newtab-intro-mask"); >+ is(intro.style.opacity, 1, "intro automatically shown on preloaded opening"); >+ is(getContentDocument().getElementById("newtab-intro-header").innerHTML, >+ "New Tab got an update!", "we show the update intro."); >+ is(Services.prefs.getBoolPref(INTRO_PREF), true, "INTRO_PREF stays true"); >+ is(Services.prefs.getBoolPref(UPDATE_INTRO_PREF), true, "newtab remembers that the update intro was show"); >+ >+ // Test clicking the 'next' and 'back' buttons. >+ let buttons = getContentDocument().getElementById("newtab-intro-buttons").getElementsByTagName("input"); >+ let progress = getContentDocument().getElementById("newtab-intro-numerical-progress"); >+ let back = buttons[0]; >+ let next = buttons[1]; >+ >+ synthesizeNativeMouseLDown(next); >+ synthesizeNativeMouseLUp(next).then(() => { >+ is(progress.getAttribute("page"), 1, "we get to the 2nd page"); >+ is(intro.style.opacity, 1, "intro visible"); >+ }, Cu.reportError); >+ >+ synthesizeNativeMouseLDown(next); >+ synthesizeNativeMouseLUp(next).then(() => { >+ is(progress.getAttribute("page"), 2, "we get to the 3rd page"); >+ is(intro.style.opacity, 1, "intro visible"); >+ }, Cu.reportError); >+ >+ synthesizeNativeMouseLDown(back); >+ synthesizeNativeMouseLUp(back).then(() => { >+ is(progress.getAttribute("page"), 1, "go back to 2nd page"); >+ is(intro.style.opacity, 1, "intro visible"); >+ }, Cu.reportError); >+ >+ synthesizeNativeMouseLDown(back); >+ synthesizeNativeMouseLUp(back).then(() => { >+ is(progress.getAttribute("page"), 0, "go back to 1st page"); >+ is(intro.style.opacity, 1, "intro visible"); >+ }, Cu.reportError); >+ >+ synthesizeNativeMouseLDown(back); >+ synthesizeNativeMouseLUp(back).then(() => { >+ is(progress.getAttribute("page"), 0, "another back will 'skip tutorial'"); >+ is(intro.style.opacity, 0, "intro exited"); >+ }, Cu.reportError); > } >diff --git a/browser/locales/en-US/chrome/browser/newTab.properties b/browser/locales/en-US/chrome/browser/newTab.properties >--- a/browser/locales/en-US/chrome/browser/newTab.properties >+++ b/browser/locales/en-US/chrome/browser/newTab.properties >@@ -21,19 +21,41 @@ newtab.sponsored.explain=This tile is be > # LOCALIZATION NOTE(newtab.suggested.explain): %1$S will be replaced inline by > # the (X) block icon. %2$S will be replaced by an active link using string > # newtab.learn.link as text. > newtab.suggested.explain=This site is suggested to you by Mozilla. You can remove it at any time by clicking the %1$S button. %2$S > # LOCALIZATION NOTE(newtab.enhanced.explain): %1$S will be replaced inline by > # the gear icon used to customize the new tab window. %2$S will be replaced by > # an active link using string newtab.learn.link as text. > newtab.enhanced.explain=A Mozilla partner has visually enhanced this tile, replacing the screenshot. You can turn off enhanced tiles by clicking the %1$S button for your preferences. %2$S >-# LOCALIZATION NOTE(newtab.intro.paragraph1): %1$S will be replaced inline by >-# active link using string newtab.learn.link as text. >-newtab.intro.paragraph1=When you open a new tab, youâll see tiles from the sites you frequently visit, along with tiles that we think might be of interest to you. Some of these tiles may be sponsored by Mozilla partners. Weâll always indicate to you which tiles are sponsored. %1$S > # LOCALIZATION NOTE(newtab.intro.paragraph2): %1$S will be replaced inline by > # active link using string newtab.privacy.link as text. > newtab.intro.paragraph2=In order to provide this service, Mozilla collects and uses certain analytics information relating to your use of the tiles in accordance with our %1$S. >-# LOCALIZATION NOTE(newtab.intro.paragraph3): %1$S will be replaced inline by >-# the gear icon used to customize the new tab window. >-newtab.intro.paragraph3=You can turn off the tiles feature by clicking the %1$S button for your preferences. >+# LOCALIZATION NOTE(newtab.intro.paragraph4): %1$S will be replaced inline by >+# the gear icon used to customize the new tab window. %2$S will be replaced by >+# newtab.intro.controls as text >+newtab.intro.paragraph4=You can turn off this feature by clicking the gear (%1$S) button and selecting "Show blank page" in the %2$S menu. >+newtab.intro.paragraph5=New Tab will show the sites you visit most frequently, along with sites we think might be of interest to you. To get started, you'll see several sites from Mozilla. >+# LOCALIZATION NOTE(newtab.intro.paragraph6): %1$S will be replaced by >+# newtab.intro.delete as bold text. %2$S will be replaced by >+# newtab.intro.pin as bold text >+newtab.intro.paragraph6=You can %1$S or %2$S any site by using the controls available on rollover. >+newtab.intro.paragraph7=Some of the sites you will see may be suggested by Mozilla and may be sponsored by a Mozilla partner. We'll always indicate which sites are sponsored. >+# LOCALIZATION NOTE(newtab.intro.paragraph8): %1$S will be replaced inline by >+# an active link using string newtab.learn.link as text. >+newtab.intro.paragraph8=Firefox will only show sites that most closely match your interests on the Web. %1$S >+newtab.intro.paragraph9=Now when you open New Tab, you'll also see sites we think might be interesting to you. >+# LOCALIZATION NOTE(newtab.intro.controls): the controls in the gear icon >+# menu for customizing the new tab window. Used in newtab.intro.paragraph4 >+newtab.intro.controls=New Tab Controls > newtab.learn.link=Learn more⦠> newtab.privacy.link=Privacy Notice >+newtab.learn.link2=More about New Tab >+newtab.privacy.link2=About your privacy >+newtab.intro.delete=delete >+newtab.intro.pin=pin >+newtab.intro.header.welcome=Welcome to New Tab on Firefox! >+newtab.intro.header.update=New Tab got an update! >+newtab.intro.skip=Skip this >+newtab.intro.continue=Continue tour >+newtab.intro.back=Back >+newtab.intro.next=Next >+newtab.intro.gotit=Got it!

oops giant comment O_O

Comment on attachment 8600022 [details] [diff] [review] Part 1: v4: Onboarding UI without the tile images Review of attachment 8600022 [details] [diff] [review]: ----------------------------------------------------------------- Nice job! ::: browser/base/content/newtab/intro.js @@ +55,5 @@ > + /** > + * The paragraphs & buttons to show on each page in the intros. > + */ > + _introPages: { > + "welcome": [[0,1],[2,3],[4,5]], Could you please add a comment explaining these structures? @@ +79,5 @@ > + }, > + > + _goToPage: function(pageNum) { > + let buttonStrings = { > + "continue": this._newTabString("intro.continue"), Nit: You could replace this table with simply this._newTabString("intro." + arg), below. @@ +111,5 @@ > + return `<strong> ${str} </strong>` > + }, > + > + _link: function(url, text) { > + return '<a href="' + url + '" target="_blank">' + text + "</a>" Nit: Why a template string for _bold but concatenation for _link? Not a big deal, but it would be nice for them to be consistent. ::: browser/base/content/newtab/newTab.css @@ +551,5 @@ > + width: 100%; > + height: 100%; > + background: #424F5A; > + z-index:100; > + background-color: rgba(66,79,90,0.95);; Nit: extra semicolon

Marina, here are my comments: 1.) Layover dropshadow opacity should be set to "multiply", not solid. It currently looks like a glow instead of a darker shadow. 2.) The orange progress fill (under "1/3, 2/3, 3/3") looks faint or partially transparent. It should be solid #FF9500. Please confirm or fix. That's all for now!

3.) Please bold the appropriate words depicted in the comps here: https://www.dropbox.com/sh/qytsdfoqide7095/AAA42KJdgI-DqRin3hYl3J1ja?dl=0

> 1.) Layover dropshadow opacity should be set to "multiply", not solid. It > currently looks like a glow instead of a darker shadow. Aaron, would you be able to specify in CSS the styling for this? I did "box-shadow: 0px 2px 4px #C1C1C1;" but it doesn't look like box-shadow has a multiply property. Thanks!

Marina, the more I think about it, the solution is super simple: just make the shadow #000000 / 70% transparency. Thank you!

This patch goes on top of part 1 and part 2 and it replaces hardcoded strings with localizable strings.

Comment on attachment 8602885 [details] [diff] [review] [strings] Part 3: v1: Change to localizable strings >+# LOCALIZATION NOTE(newtab.intro.header.welcome): %1$S will be replaced by >+# newtab.intro.firefox as bold text. >+newtab.intro.header.welcome=Welcome to New Tab on %1$S >+newtab.intro.firefox=Firefox! Probably want to reuse brandShortName defined elsewhere. And move the "!" to .welcome ? http://mxr.mozilla.org/mozilla-central/source/browser/base/content/webrtcIndicator.js#12 15 let brand = Services.strings.createBundle("chrome://branding/locale/brand.properties"); 16 let brandShortName = brand.GetStringFromName("brandShortName");

Comment on attachment 8602390 [details] [diff] [review] Part 2: v1: Add onboarding images Review of attachment 8602390 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/newtab/intro.js @@ +181,5 @@ > + image.classList.add("newtab-intro-image-customize"); > + break; > + case this._imageTypes.PIN_REMOVE: > + imageClass = "-hover"; > + case this._imageTypes.SUGGESTED: Please add a comment saying that the fall-through is intentional. @@ +197,5 @@ > + ' <input type="button" class="newtab-control newtab-control-pin"/>' + > + ' <input type="button" class="newtab-control newtab-control-block"/>' + (imageClass ? "" : > + ' <span class="newtab-sponsored">SUGGESTED</span>') + > + ' </div>' + > + '</div>'; Please add a break here to prevent accidentally falling through if we add another case in the future.

(In reply to Ed Lee :Mardak from comment #31) > Comment on attachment 8602885 [details] [diff] [review] > [strings] Part 3: v1: Change to localizable strings > > >+# LOCALIZATION NOTE(newtab.intro.header.welcome): %1$S will be replaced by > >+# newtab.intro.firefox as bold text. > >+newtab.intro.header.welcome=Welcome to New Tab on %1$S > >+newtab.intro.firefox=Firefox! > Probably want to reuse brandShortName defined elsewhere. And move the "!" to > .welcome ? Ed's right.

https://hg.mozilla.org/integration/fx-team/rev/5b45e43dec40 https://hg.mozilla.org/integration/fx-team/rev/cb5842311d43 https://hg.mozilla.org/integration/fx-team/rev/70121f99d5f1

All backed out in https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=9c709ae78ec4 for bc1 orange: https://treeherder.mozilla.org/logviewer.html#?job_id=3007032&repo=fx-team

I unbacked out the SUGGESTED string https://hg.mozilla.org/integration/fx-team/rev/5751177e9259

Backout: https://hg.mozilla.org/integration/fx-team/rev/3cb8a737f45b

I landed part 3 as 3a with only strings except for the one that already landed in bug 1138817 for SUGGESTED. I also updated a localization note to say brandShortName.

Here's a build from fx-team before it was backed out: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-macosx64/1431115202/firefox-40.0a1.en-US.mac.dmg On page 1/3 of update or 3/3 of intro, "Learn more..." seems to have a default link styling. Should it be the same as the styling at the bottom for "About your privacy" ? For 3/3 of update or 1/1 of intro, the menu has a pointer cursor making it seem like the menu can be clicked on. "Privacy Notice" also has default link styling. The inline gear ([gear]) icon also has a pointer cursor. For the link styling of the bottom items, they become black when visited. Not sure if that's expected.

And for those testing, you can go to about:config and toggle introShown/updateIntroShown to see the intro again. (I believe dcrobot said we don't want a menu item to trigger the intro again.)

(In reply to Ed Lee :Mardak from comment #41) > I landed part 3 as 3a with only strings except for the one that already > landed in bug 1138817 for SUGGESTED. I also updated a localization note to > say brandShortName. Backed out in https://hg.mozilla.org/integration/fx-team/rev/153c1531b908 since that removed the currently-used strings, breaking the browser_newtab_intro.js test.

I really wish we could land so many strings not so close to merge day. ># LOCALIZATION NOTE(newtab.intro.paragraph6): %1$S will be replaced by ># newtab.intro.remove as bold text. %2$S will be replaced by ># newtab.intro.pin as bold text >newtab.intro.paragraph6=You can %1$S or %2$S any site by using the controls available on rollover. >newtab.intro.remove=remove >newtab.intro.pin=pin Can you confirm that "remove" and "pin" are used solely in this string? >newtab.intro.paragraph8 = Firefox will only show sites that most closely match your interests on the Web. %1$S Why hard-coded Firefox and not brandShortName? ># LOCALIZATION NOTE(newtab.intro.header.welcome): %1$S will be replaced by ># newtab.intro.firefox as bold text. There's no such thing as "newtab.intro.firefox" in the landed strings.

Sorry for the late in nightly strings. Is there something that needs to be specially handled because the strings were backed out? (In reply to Francesco Lodolo [:flod] (UTC+2) from comment #45) > ># LOCALIZATION NOTE(newtab.intro.paragraph6): %1$S will be replaced by > ># newtab.intro.remove as bold text. %2$S will be replaced by > ># newtab.intro.pin as bold text > >newtab.intro.paragraph6=You can %1$S or %2$S any site by using the controls available on rollover. > >newtab.intro.remove=remove > >newtab.intro.pin=pin > Can you confirm that "remove" and "pin" are used solely in this string? They are, and I made it explicit by changing them to newtab.intro.paragraph6.pin/remove. > >newtab.intro.paragraph8 = Firefox will only show sites that most closely match your interests on the Web. %1$S > Why hard-coded Firefox and not brandShortName? Changed to %1$S for brandShortName and %2$S for learn more. > ># LOCALIZATION NOTE(newtab.intro.header.welcome): %1$S will be replaced by > ># newtab.intro.firefox as bold text. > There's no such thing as "newtab.intro.firefox" in the landed strings. This was fixed to brandShortName after the initial back out.

Comment on attachment 8603860 [details] [diff] [review] part 3a: v2 strings only Review of attachment 8603860 [details] [diff] [review]: ----------------------------------------------------------------- Since strings were backed out "quickly", you can land them without new IDs. You'll need to land this before merge, to avoid breaking string freeze in Aurora and get approval. Patch LGTM.

Comment on attachment 8603860 [details] [diff] [review] part 3a: v2 strings only emtwo, f? just to remind you that I renamed remove/pin to paragraph6.remove/pin and changed paragraph8 to accept brandShortName. Also, I didn't remove the paragraph1/3 to not break the current tests.

Updated tests with fixes that ran fine on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5902a1a16fa Also adjusted patches accordingly based on already landed string changes https://hg.mozilla.org/integration/fx-team/rev/f44d1033b21e https://hg.mozilla.org/integration/fx-team/rev/53c8327e4b03 https://hg.mozilla.org/integration/fx-team/rev/a0cbe2e08a48

https://hg.mozilla.org/mozilla-central/rev/f44d1033b21e https://hg.mozilla.org/mozilla-central/rev/53c8327e4b03 https://hg.mozilla.org/mozilla-central/rev/a0cbe2e08a48

Comment on attachment 8580819 [details] onboarding spec obsolete spec. new one should be attachment 8592387 [details] I believe..

We'll be uplifting this to 39 and 40, but we'll want to resolve some l10n issues first for 40, e.g., bug 1165394 and bug 1165430. For 39, we're uplifting this as en-US only.

Comment on attachment 8600022 [details] [diff] [review] Part 1: v4: Onboarding UI without the tile images >+++ b/browser/base/content/newtab/intro.js >+ _introPages: { >+ "welcome": [[0,1],[2,3],[4,5]], >+ "update": [[6,5],[4,3],[0,1]], >+ for (let i = 1; i <= MAX_PARAGRAPH_ID; i++) { >+ try { >+ this._paragraphs.push(this._newTabString("intro.paragraph" + i, substringMappings[i])) >+ } catch (ex) { >+ // Paragraph with this ID doesn't exist so continue >+++ b/browser/locales/en-US/chrome/browser/newTab.properties >-newtab.intro.paragraph1=When you open a new tab, youâll see tiles from the sites you frequently visit, along with tiles that we think might be of interest to you. Some of these tiles may be sponsored by Mozilla partners. Weâll always indicate to you which tiles are sponsored. %1$S >-newtab.intro.paragraph3=You can turn off the tiles feature by clicking the %1$S button for your preferences. Turns out this logic is somewhat dangerous in that it relies on the newTab.properties file. I was unremoving the paragraph1/3 for aurora uplift, and this changed the logic of which strings get shown. _introPages.welcome/update are using the compacted/id-skipped array, but safer would probably be to have this._paragraphs be a sparse array that matches the ids of the .properties.

For aurora, the 2 paragraphs will be unremoved: https://hg.mozilla.org/mozilla-central/rev/a0cbe2e08a48#l3.1

Comment on attachment 8610011 [details] [diff] [review] part 4: v1 (aurora only) Review of attachment 8610011 [details] [diff] [review]: ----------------------------------------------------------------- Nice, thanks Ed

Comment on attachment 8600022 [details] [diff] [review] Part 1: v4: Onboarding UI without the tile images > showIfNecessary: function() { > if (!Services.prefs.getBoolPref(PREF_INTRO_SHOWN)) { >- Services.prefs.setBoolPref(PREF_INTRO_SHOWN, true); >+ this._onboardingType = WELCOME; >+ this.showPanel(); >+ } else if (!Services.prefs.getBoolPref(PREF_UPDATE_INTRO_SHOWN) && DirectoryLinksProvider.locale == "en-US") { >+ this._onboardingType = UPDATE; > this.showPanel(); > } >+ Services.prefs.setBoolPref(PREF_INTRO_SHOWN, true); >+ Services.prefs.setBoolPref(PREF_UPDATE_INTRO_SHOWN, true); emtwo, this logic doesn't seem quite right for non en-US users who might only be seeing the old intro message. These users should have UPDATE kept false until they run a version of Firefox that can show the new intro/update.

Verified on nightly 2015-05-24 os x 1) create new profile 2) see onboarding with the desired 3 pages starting with "Welcome to New Tab on Nightly" 3) tested the 4 links (2 on bottom, 1 on page 1, 1 on page 2) 4) can go forwards/back/skip 5) toggled browser.newtabpage.updateIntroShown -> false from about:config 6) see onboarding with the 3 pages starting with "New Tab got an update!" 7) checks links and functionality as in step 3,4

Comment on attachment 8610020 [details] [diff] [review] for aurora (Mardak will land) Approval Request Comment: See bug 1140185 comment 11

(In reply to Ed Lee :Mardak from comment #59) > Comment on attachment 8600022 [details] [diff] [review] > Part 1: v4: Onboarding UI without the tile images > > > showIfNecessary: function() { > > if (!Services.prefs.getBoolPref(PREF_INTRO_SHOWN)) { > >- Services.prefs.setBoolPref(PREF_INTRO_SHOWN, true); > >+ this._onboardingType = WELCOME; > >+ this.showPanel(); > >+ } else if (!Services.prefs.getBoolPref(PREF_UPDATE_INTRO_SHOWN) && DirectoryLinksProvider.locale == "en-US") { > >+ this._onboardingType = UPDATE; > > this.showPanel(); > > } > >+ Services.prefs.setBoolPref(PREF_INTRO_SHOWN, true); > >+ Services.prefs.setBoolPref(PREF_UPDATE_INTRO_SHOWN, true); > emtwo, this logic doesn't seem quite right for non en-US users who might > only be seeing the old intro message. These users should have UPDATE kept > false until they run a version of Firefox that can show the new intro/update. Ed, good point. I have a little patch here that should address this problem

Comment on attachment 8610020 [details] [diff] [review] for aurora (Mardak will land) approval-mozilla-aurora+ granted in bug 1140185 comment 14

Verified fixed on latest Aurora, build ID: 20150528004000. Tested on Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit.