Closed Bug 1340501 Opened 7 years ago Closed 7 years ago

Incorrect text in sideloading-install pop-up for several types of add-ons

Categories

(Toolkit :: Add-ons Manager, defect, P3)

54 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla54
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- unaffected
firefox54 --- verified

People

(Reporter: vtamas, Assigned: aswan)

References

Details

(Whiteboard: triaged)

Attachments

(2 files)

[Affected versions]:
Firefox 54.0a1 (2017-02-16)

[Affected platforms]:
Windows 10 64-bit
Ubuntu 16.04 32-bit

[Notes]:
This issue reproduces for:
 - Legacy add-ons
 - Webextensions without permissions
 - Webextensions that have only non-promptable permissions declared 


[Steps to reproduce]:
1.Launch Firefox with clean profile.
2.Create extensions.webextPermissionPrompts and set it to true.
3.Create xpinstall.signatures.dev-root and set it to true.
4.Restart the browser.
5.Install via sideloading method one of the attached add-on. 
6.Go to Panel Menu [≡] and click on sideloading notification.


[Expected Results]:
“Please review this add-on’s permissions request” fragment should be removed from sideloading-installation doorhanger.

[Actual Results]:
- It is requested to approve the add-on’s permissions even if this is not applicable for several situations.
- See screenshot: https://www.screencast.com/t/VkryxoIh

[Additional Notes]:
I am attaching 3 add-on types for which this issue is reproducible.
Attached file add-ons.zip
Forgot to attach the add-ons.
Scott, can you provide text for the sideloading notification if the extension being added doesn't actually have any promptable permissions?
Flags: needinfo?(sdevaney)
I'm unclear why in this case we'd show the doorhanger at all? Why message anything related to permissions if there are none?
Flags: needinfo?(sdevaney)
(In reply to sdevaney from comment #3)
> I'm unclear why in this case we'd show the doorhanger at all? Why message
> anything related to permissions if there are none?

Its not stricly related to permissions, this is the new sideloading flow that replaced the old about:newaddon page.  So something outside of the browser dropped the extension into the user's profile, we still want the user to take an explicit action to agree to enable the extension.
Flags: needinfo?(sdevaney)
Okay, what if we made a slight adjustment to what's on that doorhanger?:

[name of add-on] added

Another program on your computer installed an add-on that may affect your browser. Please choose to Enable or Cancel (to leave it disabled).
Flags: needinfo?(sdevaney)
Blocks: 1342142
No longer blocks: 1342142
Blocks: 1342142
Assignee: nobody → aswan
Priority: -- → P3
Whiteboard: triaged
Attachment #8842489 - Flags: review?(florian)
Comment on attachment 8842489 [details]
Bug 1340501 Fix sideloading notification with no promptable permissions

https://reviewboard.mozilla.org/r/116308/#review117914

Looks reasonable; I would likely have r+'ed it without the obvious typo in the string that makes me doubt you tested this.

::: browser/base/content/test/webextensions/head.js:167
(Diff revision 1)
> + *        true if the url is correct.
> + * @param {array} permissions
> + *        The expected entries in the permissions list.  Each element
> + *        in this array is itself a 2-element array with the string key
> + *        for the item (e.g., "webextPerms.description.foo") and an
> + *        optional formatting parameter.

If that second parameter is optional, can we omit it instead of having these ', null' in most callers?

::: browser/locales/en-US/chrome/browser/browser.properties:65
(Diff revision 1)
>  # when the extension is side-loaded.
>  # %S is replaced with the localized name of the extension being installed.
>  # Note, this string will be used as raw markup. Avoid characters like <, >, &
>  webextPerms.sideloadHeader=%S added
>  webextPerms.sideloadText2=Another program on your computer installed an add-on that may affect your browser. Please review this add-on’s permissions requests and choose to Enable or Cancel (to leave it disabled).
> +webextPerms.sideloadTextNoPerms=Another program on your computer installed an add-on that may affect your browser.  Please choose to Enable or Cancel (to levae it disabled).

- We don't do the double space thing after a .
- typo on 'levae'

::: browser/modules/ExtensionsUI.jsm:291
(Diff revision 1)
> +      result.header = bundle.formatStringFromName("webextPerms.sideloadHeader", [addonName], 1);
> +      if (result.msgs.length == 0) {
> +        result.text = bundle.GetStringFromName("webextPerms.sideloadTextNoPerms");
> +      } else {
> +        result.text = bundle.GetStringFromName("webextPerms.sideloadText2");
> +      }

nit: I would write this with a little less code duplication:

let stringId = "webextPerms.sideloadText2";
if (results.msg.length == 0) {
  stringId = ...
}
result.text = bundle.GetStringFromName(stringId);

but that may just reflect my own personal preferences.
Attachment #8842489 - Flags: review?(florian) → review-
Comment on attachment 8842489 [details]
Bug 1340501 Fix sideloading notification with no promptable permissions

https://reviewboard.mozilla.org/r/116308/#review117914

> - We don't do the double space thing after a .
> - typo on 'levae'

Whoops, the double space thing is such a habit its hard to break.
Comment on attachment 8842489 [details]
Bug 1340501 Fix sideloading notification with no promptable permissions

https://reviewboard.mozilla.org/r/116308/#review118294
Attachment #8842489 - Flags: review?(florian) → review+
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8f70ae357674
Fix sideloading notification with no promptable permissions r=florian
https://hg.mozilla.org/mozilla-central/rev/8f70ae357674
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Verified fixed on Firefox 54.0a1 (2017-03-06/07) under Windows 10 64-bit, Ubuntu 16.04 32-bit and Mac OS X 10.12.1. The following text is successfully displayed according to Comment 5 for leacy add-ons and webextensions with non-promoptable permissions or no permissions: "Another program on your computer installed an add-on that may affect your browser. Please choose to Enable or Cancel (to leave it disabled)": https://www.screencast.com/t/k3ChrwNd
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.