Should add the Add-on tour in the onBoarding overlay

I'd like to help land this

By discussion the working item changed yesterday, so I'll hang out a few remaining working tours for Fred. This is a WIP patch for 6 tours: sync, overlay, one-off search, add-on, private browsing, and set default browser. It's an earlier version, we still need to change it to met l10n and the button listeners based on the architecture landed in m-c.

bug 1354707 comment 14 provide the new strings [2: Add-ons] Navigation link: Add-ons Headline: Add more functionality. Body: Add-ons expand Firefox’s built-in features, so Firefox works the way you do. Compare prices, check the weather or express your personality with a custom theme. Button: Show Add-ons in Menu

Please ignore `Polyfill - Add private browsing and search tour to onboarding overlay.`, which will be replaced after `Bug 1357046 - Should add the Private Browsing tour in the onBoarding overlay` is landed.

Comment on attachment 8876555 [details] Bug 1357029 - Should add the Add-on tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147894/#review152262 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:17 (Diff revision 2) > onboarding.tour-private-browsing.title=Browse by yourself. > onboarding.tour-private-browsing.description=There’s no reason to share your online life with trackers every time you browse. Want to keep something to yourself? Use Private Browsing with Tracking Protection. > onboarding.tour-private-browsing.button=Show Private Browsing in Menu > +onboarding.tour-addons=Add-ons > +onboarding.tour-addons.title=Add more functionality. > +onboarding.tour-addons.description=Add-ons expand %S’s built-in features, so %S works the way you do. Compare prices, check the weather or express your personality with a custom theme. Nit: Please add one comment like what was done on "onboarding.tour-title" saying %S is brandShortName. This would help doing tranlation, thanks.

Comment on attachment 8876562 [details] Bug 1357029 - Should add the Customize Firefox tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147904/#review152266 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:21 (Diff revision 2) > onboarding.tour-addons.title=Add more functionality. > onboarding.tour-addons.description=Add-ons expand %S’s built-in features, so %S works the way you do. Compare prices, check the weather or express your personality with a custom theme. > onboarding.tour-addons.button=Show Add-ons in Menu > +onboarding.tour-customize=Customize > +onboarding.tour-customize.title=Do things your way. > +onboarding.tour-customize.description=Drag, drop, and reorder %S’s toolbar and menu to fit your needs. You can even select a compact theme to give websites more room. Nit: Please add one comment like what was done on "onboarding.tour-title" saying %S is brandShortName. This would help doing tranlation, thanks.

Comment on attachment 8876564 [details] Bug 1357029 - Should add the Default Browser tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147906/#review152268 Thanks for adding these 3 tours. ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:25 (Diff revision 2) > onboarding.tour-customize.title=Do things your way. > onboarding.tour-customize.description=Drag, drop, and reorder %S’s toolbar and menu to fit your needs. You can even select a compact theme to give websites more room. > onboarding.tour-customize.button=Show Customize in Menu > +onboarding.tour-default-browser=Default Browser > +onboarding.tour-default-browser.title=We’re there for you. > +onboarding.tour-default-browser.description=Love %S? Set it as your default browser. Then when you open a link from another application, %S has you covered. Nit: Please add one comment like what was done on "onboarding.tour-title" saying %S is brandShortName. This would help doing tranlation, thanks.

Comment on attachment 8876564 [details] Bug 1357029 - Should add the Default Browser tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147906/#review152310 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:27 (Diff revision 2) > onboarding.tour-customize.button=Show Customize in Menu > +onboarding.tour-default-browser=Default Browser > +onboarding.tour-default-browser.title=We’re there for you. > +onboarding.tour-default-browser.description=Love %S? Set it as your default browser. Then when you open a link from another application, %S has you covered. > +onboarding.tour-default-browser.button=Open Default Browser Settings > +onboarding.tour-default-browser.win7.button=Make Firefox Your Default Browser Please add comment explaining why we have two strings for the button label

> > +onboarding.tour-default-browser.button=Open Default Browser Settings > > +onboarding.tour-default-browser.win7.button=Make Firefox Your Default Browser > > Please add comment explaining why we have two strings for the button label Hi Michael, do you have some comment suggestion to explain why we need win7 specific button string?

(In reply to Fred Lin [:gasolin] from comment #21) > > > +onboarding.tour-default-browser.button=Open Default Browser Settings > > > +onboarding.tour-default-browser.win7.button=Make Firefox Your Default Browser > > > > Please add comment explaining why we have two strings for the button label > > > Hi Michael, do you have some comment suggestion to explain why we need win7 > specific button string? Because in Windows 7 we can directly set Firefox as the default browser. We can't do that in Windows 8, 10 or on Mac.

Comment on attachment 8876564 [details] Bug 1357029 - Should add the Default Browser tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147906/#review152540 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:28 (Diff revision 3) > +onboarding.tour-default-browser.button=Open Default Browser Settings > +onboarding.tour-default-browser.win7.button=Make Firefox Your Default Browser Please add a localization note describing in which cases these buttons are used.

Comment on attachment 8876555 [details] Bug 1357029 - Should add the Add-on tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147894/#review152536 ::: browser/extensions/onboarding/content/onboarding.js:232 (Diff revision 3) > + div.querySelector("#onboarding-tour-addons-description").textContent = > + this._bundle.formatStringFromName("onboarding.tour-addons.description", > + [BRAND_SHORT_NAME, BRAND_SHORT_NAME], 2); Rather than passing BRAND_SHORT_NAME twice you can use %1$S in the string to use the first argument twice. Common style is to align the arguments to the start of the arguments in the line above. This line is a little long, break it up with an intermediate variable. These comments apply to all three patches.

Comment on attachment 8876562 [details] Bug 1357029 - Should add the Customize Firefox tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147904/#review152542 See the note from the earlier patch

Comment on attachment 8876555 [details] Bug 1357029 - Should add the Add-on tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147894/#review152728 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:16 (Diff revision 4) > onboarding.tour-private-browsing.title=Browse by yourself. > onboarding.tour-private-browsing.description=There’s no reason to share your online life with trackers every time you browse. Want to keep something to yourself? Use Private Browsing with Tracking Protection. > onboarding.tour-private-browsing.button=Show Private Browsing in Menu > +onboarding.tour-addons=Add-ons > +onboarding.tour-addons.title=Add more functionality. > +# LOCALIZATION NOTE(onboarding.tour-addons.description): This string will be used in the addon tour description. %1$S is brandShortName nit: addon -> add-on

Comment on attachment 8876564 [details] Bug 1357029 - Should add the Default Browser tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147906/#review152732 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:28 (Diff revision 4) > onboarding.tour-customize.button=Show Customize in Menu > +onboarding.tour-default-browser=Default Browser > +onboarding.tour-default-browser.title=We’re there for you. > +# LOCALIZATION NOTE(onboarding.tour-default-browser.description): This string will be used in the default browser tour description. %1$S is brandShortName > +onboarding.tour-default-browser.description=Love %1$S? Set it as your default browser. Then when you open a link from another application, %1$S has you covered. > +# LOCALIZATION NOTE(onboarding.tour-default-browser.win7.button): This string will be used in the default browser tour button. Maybe better: Label for a button to open the default browser settings in operative systems, like macOS or Win10, where it's not possible to set the default browser directly. ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:30 (Diff revision 4) > +onboarding.tour-default-browser.title=We’re there for you. > +# LOCALIZATION NOTE(onboarding.tour-default-browser.description): This string will be used in the default browser tour description. %1$S is brandShortName > +onboarding.tour-default-browser.description=Love %1$S? Set it as your default browser. Then when you open a link from another application, %1$S has you covered. > +# LOCALIZATION NOTE(onboarding.tour-default-browser.win7.button): This string will be used in the default browser tour button. > +onboarding.tour-default-browser.button=Open Default Browser Settings > +# LOCALIZATION NOTE(onboarding.tour-default-browser.win7.button): This string will be used in the default browser tour button. Windows 7 allow directly set Firefox as the default browser. typo: allow -> allows Label for a button to set directly Firefox as default browser, only used on Windows 7.

Comment on attachment 8876555 [details] Bug 1357029 - Should add the Add-on tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147894/#review152730 ::: browser/extensions/onboarding/content/onboarding.js:231 (Diff revision 4) > itemsFrag.appendChild(li); > // Dynamically create tour pages > let div = tour.getPage(this._window); > > + switch (tour.id) { > + case "onboarding-tour-addons": Note that the latest patch is going to use formatStringFromName for every description so the logic here will no longer required. Please remember to rebase before land.

Comment on attachment 8876562 [details] Bug 1357029 - Should add the Customize Firefox tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147904/#review152754 ::: browser/extensions/onboarding/content/onboarding.js:263 (Diff revision 4) > this._bundle.formatStringFromName("onboarding.tour-addons.description", [BRAND_SHORT_NAME], 1); > break; > + case "onboarding-tour-customize": > + div.querySelector("#onboarding-tour-customize-description").textContent = > + this._bundle.formatStringFromName("onboarding.tour-customize.description", [BRAND_SHORT_NAME], 1); > + break; Same as the addons case. Please remember to rebase before land.

Comment on attachment 8876564 [details] Bug 1357029 - Should add the Default Browser tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147906/#review152762 ::: browser/extensions/onboarding/content/onboarding.js:288 (Diff revision 4) > this._bundle.formatStringFromName("onboarding.tour-addons.description", [BRAND_SHORT_NAME], 1); > break; > + case "onboarding-tour-default-browser": > + div.querySelector("#onboarding-tour-default-browser-description").textContent = > + this._bundle.formatStringFromName("onboarding.tour-default-browser.description", [BRAND_SHORT_NAME], 1); > + break; Same as the addons case. Please remember to rebase before land.

Comment on attachment 8876555 [details] Bug 1357029 - Should add the Add-on tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147894/#review153128

Comment on attachment 8876562 [details] Bug 1357029 - Should add the Customize Firefox tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147904/#review153132

Comment on attachment 8876564 [details] Bug 1357029 - Should add the Default Browser tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147906/#review153140 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:28 (Diff revision 5) > +# LOCALIZATION NOTE(onboarding.tour-default-browser.button): Label for a button to open the default browser settings in OS, like macOS or Win10, where it's not possible to set the default browser directly. > +onboarding.tour-default-browser.button=Open Default Browser Settings > +# LOCALIZATION NOTE(onboarding.tour-default-browser.win7.button): Label for a button to directly set Firefox as the default browser (only Windows 7 allows that). > +onboarding.tour-default-browser.win7.button=Make Firefox Your Default Browser Phrase them like this: Label for a button to open the OS default browser settings where it's not possible to set the default browser directly (OSX, Windows 8 and higher). Label for a button to directly set the default browser (Windows 7). Add Linux to whichever of those is appropriate.

updated based on bug 1357046, 1357049, will remove the polyfill once bug 1357046, 1357049 in central. The update PR also add button actions for Customize and add-on tour. The button text and action of default browser tour are a bit different, so I'll set review in a separate patch.

push by rex's mozreview hint, thanks

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 5c0dd24ceebc -d c8497caddebb: rebasing 401976:5c0dd24ceebc "Bug 1357029 - Should add the Add-on tour in the onBoarding overlay;r=Fischer,mossop,rexboy" merging browser/extensions/onboarding/content/onboarding.css merging browser/extensions/onboarding/content/onboarding.js merging browser/extensions/onboarding/locales/en-US/onboarding.properties warning: conflicts while merging browser/extensions/onboarding/locales/en-US/onboarding.properties! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment on attachment 8876564 [details] Bug 1357029 - Should add the Default Browser tour in the onBoarding overlay; https://reviewboard.mozilla.org/r/147906/#review154128 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:43 (Diff revision 6) > +onboarding.tour-default-browser.description=Love %1$S? Set it as your default browser. Then when you open a link from another application, %1$S has you covered. > + > +# LOCALIZATION NOTE(onboarding.tour-default-browser.button): Label for a button to open the OS default browser settings where it's not possible to set the default browser directly. (OSX, Linux, Windows 8 and higher) > +onboarding.tour-default-browser.button=Open Default Browser Settings > + > +# LOCALIZATION NOTE(onboarding.tour-default-browser.win7.button): Label for a button to directly set the default browser (Windows 7). %S is brandShortName Nit: double-space here.

patch updated, the conflict is caused because extra string is added for hide overlay checkbox

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1bd20be7a42c Should add the Add-on tour in the onBoarding overlay;r=Fischer,mossop,rexboy https://hg.mozilla.org/integration/autoland/rev/288234ae0dfa Should add the Customize Firefox tour in the onBoarding overlay;r=Fischer,mossop,rexboy https://hg.mozilla.org/integration/autoland/rev/78f9b7e7a20e Should add the Default Browser tour in the onBoarding overlay;r=Fischer,mossop,rexboy

https://hg.mozilla.org/mozilla-central/rev/1bd20be7a42c https://hg.mozilla.org/mozilla-central/rev/288234ae0dfa https://hg.mozilla.org/mozilla-central/rev/78f9b7e7a20e

This feature "add the Add-on tour in the onBoarding overlay" has been implemented with Latest Nightly 56.0a1 on Ubuntu 16.04 (64 bit). Build ID : 20170620100236 User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0

This feature "add the Add-on tour in the onBoarding overlay" has been implemented with Latest Nightly 56.0a1 on Windows 10, 64 Bit! Build ID : 20170620030208 User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0 [bugday-20170621]

As per Comment 58 and Comment 59, I am marking this bug as verified fixed.