1369296 - show onboarding overlay in about:home

Status: VERIFIED FIXED

(Firefox :: New Tab Page, enhancement, P1)

Description

[[e-mail]]

we want to show onboarding overlay in about:Home based on PM(pdol)'s new request

Comment 1

[[e-mail]]

Attachment: Bug 1369296 - show onboarding overlay in about:home;

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

Comment 2

[[e-mail]]

The patch 
1. add the about:Home URL in check list, and 
2. turns `browser.onboarding.disabled` pref to `browser.onboarding.enabled` to align with other prefs (ex: browser.newtabpage.activity-stream.enabled)

Comment 3

[[e-mail]]

Comment on attachment 8873334 [details]
Bug 1369296 - show onboarding overlay in about:home;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144792/diff/1-2/

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8873334 [details]
Bug 1369296 - show onboarding overlay in about:home;

https://reviewboard.mozilla.org/r/144792/#review148886

::: browser/extensions/onboarding/content/onboarding.js:109
(Diff revision 2)
>  
>  addEventListener("load", function(evt) {
>    // Load onboarding module only when we enable it.
> -  if (content.location.href == ABOUT_NEWTAB_URL &&
> -      !Services.prefs.getBoolPref("browser.onboarding.disabled")) {
> +  if ((content.location.href == ABOUT_NEWTAB_URL ||
> +       content.location.href == ABOUT_HOME_URL) &&
> +      Services.prefs.getBoolPref("browser.onboarding.enabled")) {

Can you add a default to this getBoolPref call while you're here, just in case we want to use the system add-on in a version of Firefox without the default set.

Comment 5

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8873334 [details]
Bug 1369296 - show onboarding overlay in about:home;

https://reviewboard.mozilla.org/r/144792/#review149086

Looks good to me with one change needed.

::: browser/extensions/onboarding/content/onboarding.js:108
(Diff revision 2)
>  }
>  
>  addEventListener("load", function(evt) {
>    // Load onboarding module only when we enable it.
> -  if (content.location.href == ABOUT_NEWTAB_URL &&
> -      !Services.prefs.getBoolPref("browser.onboarding.disabled")) {
> +  if ((content.location.href == ABOUT_NEWTAB_URL ||
> +       content.location.href == ABOUT_HOME_URL) &&

Can we whitelist UITour on browser/app/permissions too?

Comment 6

[[e-mail]]

Comment on attachment 8873334 [details]
Bug 1369296 - show onboarding overlay in about:home;

https://reviewboard.mozilla.org/r/144792/#review149086

> Can we whitelist UITour on browser/app/permissions too?

Thanks for review! uitour is is already whitelist about:home on browser/app/permissions

Comment 7

[[e-mail]]

Comment on attachment 8873334 [details]
Bug 1369296 - show onboarding overlay in about:home;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144792/diff/2-3/

Comment 8

[[e-mail]]

Comment on attachment 8873334 [details]
Bug 1369296 - show onboarding overlay in about:home;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144792/diff/3-4/

Comment 9

[[e-mail]]

issue addressed, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/83fd58922970
show onboarding overlay in about:home;r=mossop,rexboy

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/83fd58922970

Comment 12

[Md. Majedul islam]

I can see onboarding overlay in about:home in latest nightly in  Ubuntu 16.04(64 Bit).

Build ID   : 20170607100302

User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

Comment 13

[Md. Rahimul Islam]

I can see onboarding overlay in about:home in latest nightly  55.0a1 (2017-06-06)(32-bit) in windows 10

Build ID   : 20170606030207

Mozilla/5.0 (Windows NT 10.0; rv:55.0) Gecko/20100101 Firefox/55.0

Comment 14

[Md. Majedul islam]

As per Comment 12 and Comment 13, I am marking this bug as verified fixed.