Add field to package.json to have cross domain content scripts

RESOLVED FIXED

Status

Add-on SDK
General
P1
normal
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

(Blocks: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Now that bug 734891 is landed and working, we can start using this new platform feature in SDK.
Cu.Sandbox now accept an array as first argument, which is a list of URI whose allow cross domain features in the created sandbox.

We should then add a field in package.json in order to automatically append this list of additional domain to every content scripts created by the SDK.
Something like:
{
  "permissions": [
    "http://mozilla.org",
    ...
  ]
}
We may name this field "content-permissions" instead as this restricted list of domain doesn't apply to the whole addon, but only content scripts.
(Assignee)

Comment 1

5 years ago
Bug 723627, would be a good example of usage for this feature.
Blocks: 723627
(In reply to Alexandre Poirot (:ochameau) from comment #0)
> We may name this field "content-permissions" instead as this restricted list
> of domain doesn't apply to the whole addon, but only content scripts.

I support content-permissions.

I assume with no permissions it defaults to not allowing you to access anywhere?
(Assignee)

Comment 3

5 years ago
(In reply to Dave Townsend (:Mossop) from comment #2)
> I assume with no permissions it defaults to not allowing you to access
> anywhere?

It means: no additional allowed domains, so no cross domain in content scripts. But content scripts will still have single domain access, the page one.

Updated

5 years ago
Priority: -- → P1
(Assignee)

Updated

5 years ago
Depends on: 791577
(Assignee)

Comment 4

5 years ago
Created attachment 662210 [details]
Pull request 579
Assignee: nobody → poirot.alex
Attachment #662210 - Flags: review?(zer0)

Updated

5 years ago
Blocks: 792479
Attachment #662210 - Flags: review?(zer0) → review+

Comment 5

5 years ago
Does this support wild card like http://*.yahoo.com/
Is this patch landed?


(In reply to p.c.iyer from comment #5)
> Does this support wild card like http://*.yahoo.com/

No it will not. I'm considering adding support for wild cards, but currently we don't have anything like that in gecko.
Also if you have good use cases that requires wild card support please let me know. I would need some good use cases to pass this feature request through.
(Assignee)

Comment 8

5 years ago
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #6)
> Is this patch landed?

No, still have to address review comments and rebase it against new layout.
Feel free to pick it up, otherwise I might be able to do that next week.
(Assignee)

Comment 9

5 years ago
Comment on attachment 662210 [details]
Pull request 579

Can you prioritize the review of the new revision I made to address issue reported by Will ?
So that we can land this branch and avoid me to rebase it again.
Attachment #662210 - Flags: review+ → review?(zer0)
Hi Alex, I did the 2nd round of review, there are a couple a comments to addresses / replied.
Depends on: 820170
Alex, I'm going to remove the review flag because I have this bug in my queue since a while and I believe we can't do anything more until bug 820170 is fixed: is it correct?

Please, feel free to re-add me anytime as soon as it's ready to be reviewed again!
Flags: needinfo?(poirot.alex)
Attachment #662210 - Flags: review?(zer0)
(Assignee)

Comment 12

4 years ago
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #11)
> Alex, I'm going to remove the review flag because I have this bug in my
> queue since a while and I believe we can't do anything more until bug 820170
> is fixed: is it correct?

Yes.
Flags: needinfo?(poirot.alex)
The per-window private-browsing flag adds a key to package.json called "permissions" which lists the set of permissions the add-on needs (currently only "private-browsing").

Should this content-permissions key live under here as well? Maybe as "cross-domain-content"?
+1, this was our thinking about how to handle explicitly declaring super-powers for an add-on.

Updated

4 years ago
Duplicate of this bug: 853346
Alex, there may be (some or entirely) overlap in bug 715470
(Assignee)

Comment 17

4 years ago
I just pushed a rebased version that now uses "cross-domain-content" attribute in "permissions" package.json's field.
(Assignee)

Comment 18

4 years ago
Comment on attachment 662210 [details]
Pull request 579

Matteo, Do you still r+ this patch?
I'd like to close this bug and let you guys continue working on eventual followups.
Attachment #662210 - Flags: review?(zer0)
Alex, if it's running all the tests it's r+ for me. Just a couple of small things:

1. in some comments and error message you're still referring to `content-permissions` where in the code is `permissions` (that I like more), so if you could uniform the name of the property in both comments and errors' message, it'll be great.

2. The new line at the end of package.json is missing.
Attachment #662210 - Flags: review?(zer0) → review+

Comment 20

4 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/72a65fdf78d6575318dc9d015c1b62fbd35d64a0
Bug 786681 - Add field to package.json to have cross domain content script

https://github.com/mozilla/addon-sdk/commit/d96e91fc55ea78a21e5e82b6d10da12234691b1e
Merge pull request #579 from ochameau/bug/786681

Bug 786681 - Add field to package.json to have cross domain content scripts r=@ZER0
(Assignee)

Comment 21

4 years ago
Last comments addressed, branch rebased against last master, re-tested against nightly and page-mod test on fennec. 
Finally, landed!
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Blocks: 880558

Comment 22

a year ago
I've filed bug 1254435 for wildcards.

I have two strong use cases:

1. Allow a custom syntax to be used with iframes so as to allow users to get independent navigation controls displayed for each iframe. This may be of particular interest to scholars or aficionados who, while reading a book, would like to see wiki pages containing extra resources pertaining to a given paragraph shown side-by-side with each book paragraph yet while being able to independently navigate within iframes (e.g., each iframe with its back and forward buttons) if they wanted to visit resources on other websites. I filed bug 618354 hoping this would make it as a feature into Firefox, but if not, I'd like to be able to implement an add-on to replicate the desired functionality.

2. Allow websites to offer themselves as full-blown browsers. The add-on could be configured to only allow this from one site, or it could expose an API to websites to allow them to ask to serve as browsers subject to user approval. This could be useful for merely prototyping browser features or for allowing the community to design its own browser solely on HTML/JavaScript, even accepting its own add-ons, etc.

Even if neither of these could be allowed on AMO, I have been pining for years for such an ability, and would really hope this simple request could be accommodated within the SDK. I don't think it should add much at all for maintenance, and AMO could simply disallow the wildcard if it did not wish to do so. Thanks!

Comment 23

a year ago
Another specific application for the independent navigation control use case would be a stickies web app where each sticky on the page could have links which, when clicked, could be viewed within its respective sticky, yet navigated independently of pages loaded from other stickies.
You need to log in before you can comment on or make changes to this bug.