Closed Bug 1350277 Opened 7 years ago Closed 7 years ago

http://*/* and https://*/* permissions display identical descriptions

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- verified

People

(Reporter: vtamas, Assigned: andy+bugzilla, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [permissions] triaged)

Attachments

(3 files)

Attached image 2017-03-24_1136.png
[Affected versions]:
Firefox 55.0a1 (2017-03-23)
Firefox 54.0a2 (2017-03-23)
Firefox 53.0b6 (20170323121323)


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

[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.Click on “+ Add to Firefox” button to install the following webextension: https://addons-dev.allizom.org/en-US/firefox/addon/9gag-mini-permissions/

[Expected Results]:
http://*/* and https://*/* permissions displays together only one description.


[Actual Results]:
- The same description appears twice, one for each permission.
- See attached screenshot.

[Additional Notes]:
- This issue also reproduce for domains specified in "content_scripts" field.
- Here is a screenshot about how Chrome behaves using the same extension: https://www.screencast.com/t/geDlQFFVRj
Priority: -- → P3
Whiteboard: [permissions] triaged
dev doc is to use * instead of permissions for both http or https
Mentor: aswan
The fix here is pretty straightforward: use Sets instead of arrays to keep track of hosts and domains inside ExtensionsUI._buildStrings().  ie here: http://searchfox.org/mozilla-central/rev/4bd7a206dea5382c97a8a0c30beef668cc449f5b/browser/modules/ExtensionsUI.jsm#242
This also applies to hosts that appear in the "content_scripts" section of the manifest, as reported in bug 1363263
See Also: → 1373434
Assignee: nobody → amckay
Comment on attachment 8888879 [details]
Bug 1350277 remove duplicates when showing host permissons

https://reviewboard.mozilla.org/r/159904/#review169054

Nice, thanks, but can you please apply the same logic to wildcard/domain permissions?
Attachment #8888879 - Flags: review-
Thanks, updated per feedback to do the wildcard check. The xpi change just removes the signing as thats unneccesary these days and changes the manifest to have a matching http and https single host and wildcard domain.
Comment on attachment 8888879 [details]
Bug 1350277 remove duplicates when showing host permissons

https://reviewboard.mozilla.org/r/159904/#review184034

::: commit-message-e99e3:1
(Diff revision 4)
> +bug 1350277 ignore protocol when showing host permissons r?aswan

Capitalize Bug and the summary.  Although we already ignore the protocol, this is about weeding out duplciates -- update the summary to describe that more clearly?
Attachment #8888879 - Flags: review?(aswan) → review+
Keywords: checkin-needed
Autoland can't push this until all pending issues in MozReview are marked as resolved.
Flags: needinfo?(amckay)
Keywords: checkin-needed
Done.
Flags: needinfo?(amckay)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2b74ec6bb1d8
remove duplicates when showing host permissons r=aswan
Keywords: checkin-needed
Backed out for failing browser-chrome's browser/base/content/test/webextensions/browser_permissions_addons_search.js:

https://hg.mozilla.org/integration/autoland/rev/9ac8940da5c3bdc417a057c8e424c6655a1c9e88

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2b74ec6bb1d84093e3367e8903aa6d94817cfa00&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130759170&repo=autoland

12:19:39     INFO -  78 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | Permissions list has 0 entries -
12:19:39     INFO -  79 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | Permissions header is hidden -
12:19:39     INFO -  80 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | Installation was cancelled -
12:19:39     INFO -  81 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | Extension is not installed -
12:19:39     INFO -  82 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | about:addons is displaying search results -
12:19:39     INFO -  83 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | Found browser_webext_permissions.xpi in search results -
12:19:39     INFO -  84 INFO Console message: [JavaScript Error: "Unparseable host permission moz-extension://ae3bb518-f006-cd4f-9386-f015a3a47afd/*" {file: "resource://gre/modules/Extension.jsm" line: 842}]
12:19:39     INFO -  formatPermissionStrings@resource://gre/modules/Extension.jsm:842:9
12:19:39     INFO -  _buildStrings@resource:///modules/ExtensionsUI.jsm:263:12
12:19:39     INFO -  observe@resource:///modules/ExtensionsUI.jsm:169:21
12:19:39     INFO -  installRemote/this.mInstall.promptHandler/<@chrome://mozapps/content/extensions/extensions.xml:679:15
12:19:39     INFO -  installRemote/this.mInstall.promptHandler@chrome://mozapps/content/extensions/extensions.xml:659:51
12:19:39     INFO -  checkPrompt/<@resource://gre/modules/addons/XPIInstall.jsm:1700:17
12:19:39     INFO -  async*checkPrompt@resource://gre/modules/addons/XPIInstall.jsm:1691:6
12:19:39     INFO -  install@resource://gre/modules/addons/XPIInstall.jsm:1444:7
12:19:39     INFO -  install@resource://gre/modules/addons/XPIInstall.jsm:2186:7
12:19:39     INFO -  downloadCompleted/<@resource://gre/modules/addons/XPIInstall.jsm:2502:9
12:19:39     INFO -  makeSafe/<@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:104:17
12:19:39     INFO -  getRepositoryAddon@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:85:5
12:19:39     INFO -  getAddon/<@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:744:9
12:19:39     INFO -  promise callback*getAddon@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:742:12
12:19:39     INFO -  getVisibleAddonForID@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:791:5
12:19:39     INFO -  downloadCompleted@resource://gre/modules/addons/XPIInstall.jsm:2479:5
12:19:39     INFO -  onStopRequest/<@resource://gre/modules/addons/XPIInstall.jsm:2425:13
12:19:39     INFO -  promise callback*onStopRequest@resource://gre/modules/addons/XPIInstall.jsm:2423:9
12:19:39     INFO -  Buffered messages logged at 12:19:39
12:19:39     INFO -  85 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | addon-webext-permissions notification shown -
12:19:39     INFO -  86 INFO TEST-PASS | browser/base/content/test/webextensions/browser_permissions_addons_search.js | notification panel open -
12:19:39     INFO -  Buffered messages finished
12:19:39    ERROR -  87 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/webextensions/browser_permissions_addons_search.js | Notification icon is correct "chrome://browser/skin/warning.svg" ~= /^jar:file:\/\/.*\/icon\.png$/ -
Flags: needinfo?(amckay)
Turns out I do have to sign this add-on, the failures were checking if the extension was signed or not. I've signed the extension and run a try build. Looks good apart from those unrelated oranges.
Flags: needinfo?(amckay)
Keywords: checkin-needed
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 cd4094992fe5 -d 9219c2f933e2: rebasing 419919:cd4094992fe5 "Bug 1350277 remove duplicates when showing host permissons r=aswan" (tip)
merging browser/base/content/test/webextensions/browser_webext_permissions.xpi
merging toolkit/components/extensions/Extension.jsm
warning: /repos/mozreview-gecko/browser/base/content/test/webextensions/browser_webext_permissions.xpi looks like a binary file.
warning: conflicts while merging browser/base/content/test/webextensions/browser_webext_permissions.xpi! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/41effc9b8c27
Remove duplicates when showing host permissons. r=aswan
(In reply to Mozilla Autoland from comment #27)
> 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 cd4094992fe5 -d 9219c2f933e2: rebasing
> 419919:cd4094992fe5 "Bug 1350277 remove duplicates when showing host
> permissons r=aswan" (tip)
> merging
> browser/base/content/test/webextensions/browser_webext_permissions.xpi
> merging toolkit/components/extensions/Extension.jsm
> warning:
> /repos/mozreview-gecko/browser/base/content/test/webextensions/
> browser_webext_permissions.xpi looks like a binary file.
> warning: conflicts while merging
> browser/base/content/test/webextensions/browser_webext_permissions.xpi!
> (edit, then use 'hg resolve --mark')
> unresolved conflicts (see hg resolve, then hg rebase --continue)

I manually imported the commit and was able to apply and push it without issue. Was Autoland trying to rebase a binary file or something here?
Flags: needinfo?(gps)
https://hg.mozilla.org/mozilla-central/rev/41effc9b8c27
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Filed bug 1400377 to track the autoland issue.
Flags: needinfo?(gps)
Attached image 2017-09-19_1518.png
I confirm that this issue is fixed on Firefox 57.0a1 (20170918220054) under Windows 10 64-bit and Mac OS X 10.12.3. There are no longer duplicated permissions displayed in webextension installation pop-up. See screenshot.
Status: RESOLVED → VERIFIED
I'm not sure what developer documentation is needed here, can you elaborate please?
Flags: needinfo?(sescalante)
(In reply to Will Bamberg [:wbamberg] from comment #34)
> I'm not sure what developer documentation is needed here, can you elaborate
> please?

I think there was a suggestion a while back that before this bug was fixed, extension authors could work around this bug by using *://foo.com/ instead of listing both http://foo.com/ and https://foo.com/
I do agree that it doesn't seem like there's much to add now that this is fixed...
Flags: needinfo?(sescalante)
Thanks aswan.

For reference: dev-doc-needed is for bugs that deliver a new feature (or fix) that will need documenting. As such we don't look at the bug until it has been marked fixed. So adding dev-doc-needed to ask for documentation of a workaround that won't be needed after that bug is fixed will not have the desired effect. In a situation like this it's better to raise a documentation bug: https://bugzilla.mozilla.org/enter_bug.cgi?product=Developer%20Documentation.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.