1436165 - Change extensionControlled strings for home page and new tab page

Status: VERIFIED FIXED

(WebExtensions :: Frontend, enhancement, P3)

Description

[Bob Silverberg [:bsilverberg]]

This is a follow-up to bug 1429593 to make changes to two existing strings.

As per the comment at https://bugzilla.mozilla.org/show_bug.cgi?id=1429593#c57, the new strings should be:

"An extension, <icon> <insert name of extension>, is controlling your New Tab page."
"An extension, <icon> <insert name of extension>, is controlling your Home page."

This will require some changes to the code as well, as currently the code assumes the string ID is the same as the settingName, but we will need to change the ID when we change the strings.

Comment 1

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1436165 - Change extensionControlled strings for home page and new tab page

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

Comment 2

[Mark Striemer [:mstriemer]]

Comment on attachment 8949121 [details]
Bug 1436165 - Change extensionControlled strings for home page and new tab page

https://reviewboard.mozilla.org/r/218524/#review224380

::: browser/components/preferences/in-content/main.js:365
(Diff revision 1)
>  
> -    handleControllingExtension(URL_OVERRIDES_TYPE, NEW_TAB_KEY);
> +    handleControllingExtension(
> +      URL_OVERRIDES_TYPE, NEW_TAB_KEY, NEW_TAB_STRING_ID);
>      let newTabObserver = {
>        observe(subject, topic, data) {
> -          handleControllingExtension(URL_OVERRIDES_TYPE, NEW_TAB_KEY);
> +          handleControllingExtension(

nit: Looks like you didn't change this but this line is indented too far

Comment 3

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8949121 [details]
Bug 1436165 - Change extensionControlled strings for home page and new tab page

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

Comment 4

[Bob Silverberg [:bsilverberg]]

Jared, I think gandalf is eagerly awaiting the landing of this, as it's blocking bug 1424682, so if you have time to look at it today that would be great, thanks.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8949121 [details]
Bug 1436165 - Change extensionControlled strings for home page and new tab page

https://reviewboard.mozilla.org/r/218524/#review225824

::: browser/components/preferences/in-content/extensionControlled.js:150
(Diff revision 2)
>    // Remove the old content from the description.
>    while (description.firstChild) {
>      description.firstChild.remove();
>    }
>  
> +  stringId = stringId || `extensionControlled.${settingName}`;

You can move this in to the function signature as a default argument for stringId.

```js
async function showControllingExtension(settingName, addon, stringId = `extensionControlled.${settingName}`) {
```

Comment 6

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8949121 [details]
Bug 1436165 - Change extensionControlled strings for home page and new tab page

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1abd8bf0d997
Change extensionControlled strings for home page and new tab page, r=jaws,mstriemer

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1abd8bf0d997

Comment 9

[CosminB]

Attachment: Bug1436165.png

I can reproduce this issue on Firefox 58.0.2 (20180206200532) under Windows 7 64-bit and Mac OS 10.13.2.

The next strings are displayed:
"An extension, <icon> <insert name of extension>, controls your New Tab page."
"An extension, <icon> <insert name of extension>, controls your Home page."

This issue is verified as fixed on Firefox 60.0a1 (20180222101929) under Windows 7 64-bit and Mac OS X 10.13.2.

Please see the attached screenshot.