-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Targeted Grant: Native Protocol Handler API for Browser Extensions #30
Conversation
This is initial devgrant draft, not the final form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for pointing them in the right direction at the start!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. Thanks for the detailed description. We started to read the documentation and are analyzing things, but here are some quick comments/questions.
- Displays user prompt, asking for confirmation if a handler should be registered | ||
- Requires unknown protocols names to be prefixed with `web+` ([MDN](https://developer.mozilla.org/en-US/docs/Web/API/Navigator/registerProtocolHandler#Permitted_schemes)) | ||
- Firefox whitelisted `ipfs://` ([bug 1428446](https://bugzilla.mozilla.org/show_bug.cgi?id=1428446)) | ||
- Chromium requires prefix `web+ipfs://` ([bug 651311](https://bugs.chromium.org/p/chromium/issues/detail?id=651311), [intent-to-implement](https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/29sFh4tTdc)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link to the intent-to-implement seems broken. Are you able to provide the correct link or at least the feature name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is https://groups.google.com/a/chromium.org/d/msg/blink-dev/29sFh4tTdcs/K4XroilVBAAJ
Probably a new list item that references to the WHATWG issue should be added here as that's what seems to be blocking the decision: whatwg/html#3998 ; Reading the comments, I don't think anything will be implemented in browsers until Chromium and Mozilla projects reach an agreement on the spec change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. I'll update the groups link and added whatwg/html#3998 to this section.
We are aware of the stalemate and hope that going browser extension route, discussing a new, generic API could provide incentive to unblock this discussion on the vendor side.
|
||
<!-- What are the acceptance criteria for each milestone and for the final deliverables? These should be as objective as possible. They will be used to determine whether or not a grantee will receive payment for work completed for a milestone. --> | ||
|
||
- API specificiation document is approved by IPFS project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: specification
Do we want to standardize this specification? Is there a place where to put WebExtensions API specifications (where does Mozilla put such specifications) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we want to add "and discussed with Mozilla and Chromium projects" here? At least it seems important to get the feedback of the project maintainers early if we want to increase the chance to get the API to be accepted upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we definitely want this API to be created with Mozilla and Chromium being in the loop.
I've added a note to "API proliferation" section.
- reuses HTTP semantics for caching, content type, headers and error codes | ||
- API implementation in form of patches for Chromium codebase | ||
- allows JS running in browser extension context to register `ipfs://` and `ipns://` protocol handlers, process every request made with them and return arbitrary bytes | ||
- released under [PL's Permissive License Stack](https://protocol.ai/blog/announcing-the-permissive-license-stack/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would need to read more about this license, but any code that is intended to land in Chromium (or WebKit and Gecko) should be under an open source license and more specifically the license (and sometimes copyright holder) suggested by the corresponding project. For Chromium it's the BSD-like one at https://chromium.googlesource.com/chromium/src.git/+/refs/heads/master/LICENSE and copyrighted "The Chromium Authors".
I guess the text of the license might also need to be a under some open license, maybe Mozilla has rules too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. By default, PL releases everything under both MIT+Apache 2.0 to ease the adoption. This grant includes contribution to a third party project – we can be flexible here. I've updated this point to allow for a license suggested by Chromium project.
|
||
- Works only when executed on matching Origin | ||
- User needs to be on `example.com` to registration to work | ||
- Displays user prompt, asking for confirmation if a handler should be registered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess such a confirmation will probably be asked during the extension installation with a native API? I don't remember exactly how WebExtensions works with this kind of permission.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Install process displays the list of permissions requested by the extension, for example on which URLs it is allowed to run etc. I imagine on the UX side provided protocol schemes could be just another item on that list (eg. ipfs://*
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be my guess too.
- API proliferation | ||
- can be enabled at a build time by browser vendors such as Brave or Edge | ||
- patches submitted to the Brave project | ||
- patches submitted to the upstream Chromium / Blink projects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, this need to be checked (as this is just a WebExtensions API) but new platform features follow https://www.chromium.org/blink/launching-features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it was unclear to me if browser extension APIs fall under "web API":
You do not need to use this process if:
- Your change does not affect web API behavior to the point that developers need to be aware of it.
[..]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any precedent for an extension API landing in Chromium but only shipping in non-Chrome browser? Might be entering some uncharted water. Hopefully can make it clearer for those following.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I don't believe any new web platform feature are landing in Chromium without being enabled in Chrome. But again, I'm not sure for web extension, I would need to talk to people who are more familiar with that.
|
||
<!-- What are the acceptance criteria for each milestone and for the final deliverables? These should be as objective as possible. They will be used to determine whether or not a grantee will receive payment for work completed for a milestone. --> | ||
|
||
- API specificiation document is approved by IPFS project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we want to add "and discussed with Mozilla and Chromium projects" here? At least it seems important to get the feedback of the project maintainers early if we want to increase the chance to get the API to be accepted upstream.
## Resources | ||
|
||
<!-- Link any resources that might be helpful for an implementer who is working on this project. --> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should numbers + id be used so that the links used above can just point to the corresponding reference in the resources section?
|
||
If possible, the API should be compatible with `manifest.json/protocol_handlers` API already present in Firefox | ||
(registering handler on extension install) but with option to omit `uriTemplate` and provide a self-hosted, | ||
programmatic handler via `chrome.protocol.registerProtocol` instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a reference link for chrome.protocol.registerProtocol
? It does not seem obvious to me what this paragraph is referring to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant future API which does not exist yet. Updated the section to make it clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think this is my main interrogation right now as without further investigation it's a bit difficult to estimate how much work is needed to implement the backend code or if there is anything we can rely on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lidel To clarify my interrogation, https://blog.ipfs.io/2019-10-08-ipfs-browsers-update mentions
Chromium OS has a raw sockets API, but it’s disabled in the Chrome browser. Brave has modified their build to whitelist those APIs for IPFS Companion to use - and this gives us superpowers compared to any other browser at this point."
So do you expect we would essentially just enable & expose these ChromiumOS APIs via a new chrome.*.registerProtocol
? Or is a more significant effort needed on the backend code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fred-wang I know this part is bit confusing, so let me elaborate: chrome.*
namespace in Chromium-based browsers is populated by both Browser Extension APIs and Chrome OS/Apps APIs, but in this grant we care only about the browser ones.
Situation looks like this:
- chrome apps: https://developer.chrome.com/apps/api_index
Avoid in the long term, as Google deprecated them and will remove support by 2022. - browser extensions: https://developer.chrome.com/extensions/api_index
These APIs were reimplemented by Firefox when they move away from XUL-based extensions. This is where we want to add the new protocol handler API
We would not change any of existing APIs onchrome.*
, we would only add a new API to that namespace because that is where other browser extension APIs are)
The new API should enable browser extension to register handler for foo://
and return arbitrary bytes as stream, and if possible, with mandatory/optional content-type (so if content type is stored in IPFS we can avoid mime-type sniffing and provide it with the response).
Examples of streaming "protocol handler API":
- https://github.com/mozilla/libdweb#protocol-api
- https://www.electronjs.org/docs/api/protocol#protocol
We did not write any specifics about the API as we don't want to be overly prescriptive, but we can add something as a starting point, if you feel it will make it easier to discuss this further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification ! That's how I had understood the Chrome API stuff, my doubt was about the backend effort/size needed for a "generic Protocol Handler API" exactly. I don't think we need the detailed API in the grant description but probably a simple sketch/example of the desired behavior like you commented here is enough, maybe saying it would be similar to the one mentioned in "Firefox Nightly + libdweb" (incidentally I suspect using modern async iterators is probably what we want). From what I get now, we don't need to add complex network stuff in the backend, we can rely on existing JS async generators or similar and the API would be gluing things together ; most of the actual network work will done by the users.
One thing that confused me was that I forgot this is "generic" and not specific to ipfs at all. Perhaps "Project Description" and "Value" sections should be tweaked a bit to highlight that it would allow people to register any (whitelisted) "foo://" protocol, and IPFS ones in particular. I think this is a strong argument to use, for the grant request and for Chromium/Mozilla review.
I'll continue discussion with my colleagues at Igalia and we are going to follow-up privately.
cc @bkardell |
#30 (review) License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
#30 (comment) License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
This is wip, needs input from Igalia
cc @autonome
PREVIEW: https://github.com/ipfs/devgrants/blob/master/targeted-grants/protocol-handler-api-for-browser-extensions.md