1414029 - Include a link to a SUMO article in extension doorhangers

Status: VERIFIED FIXED

(WebExtensions :: Frontend, defect, P4)

Description

[Mark Striemer [:mstriemer]]

The New Tab and homepage will get a doorhanger when opened for the first time if they are controlled by an extension.

This doorhanger should have a link to a SUMO article so that the user can learn more about why we are asking them to confirm this change.

We will need the SUMO article to be made before the link can be added to the doorhanger.

Comment 1

[Mark Striemer [:mstriemer]]

Do we have a page for this, Markus?

Comment 2

[Markus Jaritz [:designakt] (UX)]

There is no article on SUMO yet. Joni, can you please help us with that.

I would hope this article will inform about why one sees this message.
(an extension one added, changed the page)
(the message will be hanging off of that extensions toolbar icon, if the extension has one.)

And how to revert the change if one decides to keep it for now.
(go to preferences, find override info, remove extension; or remove/disable extension from addons manager)


Article with related content:
https://support.mozilla.org/en-US/kb/how-to-set-the-home-page


Mockup of the dialog informing about the changed homepage:
https://mozilla.invisionapp.com/share/6HCITJKP8#/244736432_Jazz_-_InContext_-_Home_-_V4_-_01_Override_Confirmation

Mockup of the dialog informing about the changed newTab:
https://mozilla.invisionapp.com/share/6HCITJKP8#/244736434_Jazz_-_InContext_-_NewTab_-_V4_-_01_Override_Confirmation

Mockup of the UI surfacing the extension controlling newTab/Homepage:
https://mozilla.invisionapp.com/share/6HCITJKP8#/242994343_Jazz_-_Preferences_-_General_-_V4_-_02_Override_NewTab___Home_Page

Joni, we can help provide screenshots if need.

Comment 3

[Joni Chan]

Thanks for looping me in. When is this shipping on Beta?

I have an article draft for your review here: https://docs.google.com/a/mozilla.com/document/d/1V3jf7mwgdA3006-5hv0dksE9cxlNjcyvcfZvwphJwp8/edit?usp=sharing

I've left a couple of questions for you as comments in the article.

Here's the URL you can hardcode in the UI: https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/extension-home

Comment 4

[Markus Jaritz [:designakt] (UX)]

(In reply to Joni Savage ("need info" me) from comment #3)
> Thanks for looping me in. When is this shipping on Beta?

I am not certain, it might be 58, or 59. Mark will know better.

Comment 5

[Markus Jaritz [:designakt] (UX)]

(In reply to Joni Savage ("need info" me) from comment #3)
> I have an article draft for your review here:
> https://docs.google.com/a/mozilla.com/document/d/1V3jf7mwgdA3006-5hv0dksE9cxlNjcyvcfZvwphJwp8/edit?usp=sharing
> 
> I've left a couple of questions for you as comments in the article.

I answered your questions and left some more comments. 
This made me realize that it will be way easier writing this as soon as everything is in beta, and you can experience the UI yourself.

Important to not for now is, that this is talking about 2 slightly different use-cases. And might deserve 2 individual URLs, even if we end up referring both to the same article. 
A) extension overrides a users newTab and they get a message when they first see the new newTab
B) extension overrides a users Homepage and they get a message when they first see the new Homepage

What do you think Joni?

Comment 6

[Mark Striemer [:mstriemer]]

This will be in 59.

There are two places we will show this link, as Markus mentioned. On the New Tab page and on the Homepage. The New Tab will land first and then the Homepage will come after.

I'll need to know if they should use the same URL or if they will use different URLs, what URL the New Tab page should link to.

Comment 7

[Alice Wyman]

I see that a "placeholder" article was created by Joni on Nov 14, 2017 but it needs content:
https://support.mozilla.org/en-US/kb/extension-changed-my-home-page/

Comment 8

[Joni Chan]

I've published the article per my conversation with @designakt. 

Here are the links you can use in the product: 
Home page: https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/extension-home
New Tab: https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/extension-home
extension-new-tab

They'll redirect to the same article which is https://support.mozilla.org/kb/extension-changed-my-new-tab-or-home-page

Comment 9

[Mark Striemer [:mstriemer]]

The two links look the same, is that expected?

When I use the in product link I get sent to https://support.mozilla.org/1/firefox/60.0a1/Darwin/en-US/extension-home which redirects to https://support.mozilla.org/en-US/extension-changed-my-new-tab-or-home-page?as=u&utm_source=inproduct which is a 404 since it is missing the `kb` part of the URL.

Comment 10

[Joni Chan]

Does the URL work for you now? I added the KB part in the target URL.

Comment 11

[Mark Striemer [:mstriemer]]

Attachment: Bug 1414029 - Include learn more link on New Tab doorhanger

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

Comment 12

[Mark Striemer [:mstriemer]]

Attachment: new-tab-rebased.mov.gif

Comment 13

[Andrew Swan [:aswan]]

Comment on attachment 8957714 [details]
Bug 1414029 - Include learn more link on New Tab doorhanger

https://reviewboard.mozilla.org/r/226660/#review232592

::: browser/components/extensions/ext-url-overrides.js:94
(Diff revision 1)
>    description.appendChild(
>      BrowserUtils.getLocalizedFragment(doc, message, addonDetails));
>  
> +  // Add the Learn more link to the description.
> +  let link = doc.createElement("label");
> +  link.setAttribute("id", "extension-new-tab-learn-more");

does this need an id?

::: browser/components/extensions/ext-url-overrides.js:97
(Diff revision 1)
> +  // Add the Learn more link to the description.
> +  let link = doc.createElement("label");
> +  link.setAttribute("id", "extension-new-tab-learn-more");
> +  link.setAttribute("class", "learnMore text-link");
> +  link.href = Services.urlFormatter.formatURL(
> +    "https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/extension-home");

Can you base this is on the app.support.baseURL preference.  ie something like
https://searchfox.org/mozilla-central/rev/44fa24847e4e73ce8932de4c8cf47559302e4ba2/browser/base/content/browser-addons.js#397

Comment 14

[Julien Cristau [:jcristau]]

This missed the branch to beta, so if it needs to make 60 it will need l10n approval.  ni?flod fyi.

Comment 15

[Francesco Lodolo [:flod]]

(In reply to Julien Cristau [:jcristau] from comment #14)
> This missed the branch to beta, so if it needs to make 60 it will need l10n
> approval.  ni?flod fyi.

Thanks for the heads up. If it has to target 60, it's OK to uplift. Sadly, I don't think it's going to be the only one.

Comment 16

[Mark Striemer [:mstriemer]]

Comment on attachment 8957714 [details]
Bug 1414029 - Include learn more link on New Tab doorhanger

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

Comment 17

[Francesco Lodolo [:flod]]

Comment on attachment 8957714 [details]
Bug 1414029 - Include learn more link on New Tab doorhanger

https://reviewboard.mozilla.org/r/226660/#review233222

::: toolkit/locales/en-US/chrome/global/extensions.properties:34
(Diff revisions 1 - 2)
>  uninstall.confirmation.button-1.label = Keep Installed
>  
>  saveaspdf.saveasdialog.title = Save As
>  
> -#LOCALIZATION NOTE (newTabControlled.message) %S is the icon and name of the extension which updated the New Tab page.
> -newTabControlled.message = An extension, %S, changed the page you see when you open a new tab.
> +#LOCALIZATION NOTE (newTabControlled.message2) %S is the icon and name of the extension which updated the New Tab page.
> +newTabControlled.message2 = An extension, %S, changed the page you see when you open a new tab.

Why are you changing the string ID? The string doesn't change, you're asking all languages to retranslate it.

Comment 18

[Francesco Lodolo [:flod]]

I'm really confused by mozreview right now. The full diff doesn't show newTabControlled.message2 as changed. 
https://reviewboard.mozilla.org/r/226660/diff/2#index_header

While the partial diff does
https://reviewboard.mozilla.org/r/226660/#review233222

But I also don't see traces of newTabControlled.message in mozilla-central (and inbound, autoland)?
https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/toolkit/locales/en-US/chrome/global/extensions.properties

Comment 19

[Mark Striemer [:mstriemer]]

That message changed in bug 1444149 which this depends on and is also targeting 60. This is what the diff should be:

changeset:   454675:a2f86d706ff7
bookmark:    1414029-newtab-link
parent:      454670:2045bbb3f7ce
user:        Mark Striemer <[e-mail]>
date:        Fri Mar 09 10:35:27 2018 -0600
trouble:     unstable
summary:     Bug 1414029 - Include learn more link on New Tab doorhanger r?aswan

diff --git a/browser/components/extensions/ext-url-overrides.js b/browser/components/extensions/ext-url-overrides.js
--- a/browser/components/extensions/ext-url-overrides.js
+++ b/browser/components/extensions/ext-url-overrides.js
@@ -89,6 +89,13 @@ async function handleNewTabOpened() {
   description.appendChild(
     BrowserUtils.getLocalizedFragment(doc, message, addonDetails));
 
+  // Add the Learn more link to the description.
+  let link = doc.createElement("label");
+  link.setAttribute("class", "learnMore text-link");
+  link.href = Services.urlFormatter.formatURLPref("app.support.baseURL") + "extension-home";
+  link.textContent = strBundle.GetStringFromName("newTabControlled.learnMore");
+  description.appendChild(link);
+
   // Setup the command handler.
   let handleCommand = async (event) => {
     if (event.originalTarget.getAttribute("anonid") == "button") {
diff --git a/toolkit/locales/en-US/chrome/global/extensions.properties b/toolkit/locales/en-US/chrome/global/extensions.properties
--- a/toolkit/locales/en-US/chrome/global/extensions.properties
+++ b/toolkit/locales/en-US/chrome/global/extensions.properties
@@ -32,3 +32,4 @@ saveaspdf.saveasdialog.title = Save As
 
 #LOCALIZATION NOTE (newTabControlled.message2) %S is the icon and name of the extension which updated the New Tab page.
 newTabControlled.message2 = An extension, %S, changed the page you see when you open a new tab.
+newTabControlled.learnMore = Learn more

Comment 20

[Francesco Lodolo [:flod]]

(In reply to Mark Striemer [:mstriemer] from comment #19)
> That message changed in bug 1444149 which this depends on and is also
> targeting 60.

Thanks, makes a lot more sense now.

Comment 21

[Mark Striemer [:mstriemer]]

This patch depends on the changes in bug 1444149.

Comment 23

[Mozilla Autoland]

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 fb52f0d1df334c8f926e2a9c2f3ad5e7e9b3edc3 -d 6f60e8f0463c: rebasing 452106:fb52f0d1df33 "Bug 1414029 - Include learn more link on New Tab doorhanger r=aswan" (tip)
merging browser/components/extensions/ext-url-overrides.js
merging toolkit/locales/en-US/chrome/global/extensions.properties
warning: conflicts while merging toolkit/locales/en-US/chrome/global/extensions.properties! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 24

[Raul Gurzau (:RaulG)]

There are conflicts while landing this patch, :mstriemer Can you please have a look?

Comment 25

[Mark Striemer [:mstriemer]]

Comment on attachment 8957714 [details]
Bug 1414029 - Include learn more link on New Tab doorhanger

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

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b641f745dad9
Include learn more link on New Tab doorhanger r=aswan

Comment 27

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/b641f745dad9

Comment 28

[Mark Striemer [:mstriemer]]

Comment on attachment 8957714 [details]
Bug 1414029 - Include learn more link on New Tab doorhanger

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: There is not a lot of information in the new tab doorhanger, this adds a "Learn more" link to explain further.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: 
  1. Install an extension that controls the New Tab, ex: Tabby Cat or Tabliss
  2. Open a new tab, verify that there is a "Learn More" link that takes the user to a SUMO article explaining what the extension has changed.
[List of other uplifts needed for the feature/fix]: bug 1444149 changes must land first
[Is the change risky?]: No
[Why is the change risky/not risky?]: String/link addition
[String changes made/needed]: Yes

Comment 29

[Mark Striemer [:mstriemer]]

I have manually verified this on Nightly.

Comment 30

[Julien Cristau [:jcristau]]

Comment on attachment 8957714 [details]
Bug 1414029 - Include learn more link on New Tab doorhanger

add link to doorhanger, l10n changes acked by flod, beta60+

Comment 31

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cf8705c4a1bf

Comment 32

[marius.santa]

Attachment: tabby_cat-0.995-an+fx-windows.xpi

Tested and reproduced in Firefox 60.0a1 (20180308100121) on Windows 10 64Bit.
Retested and verified in Firefox 61.0a1 (20180403100105) and Firefox 60.0b9 (20180402175344) on Windows 10 64Bit and MacOS 10.13.3.