Skip to content

Add protocol handlers to the explainer and spec#863

Closed
fabiorocha wants to merge 1 commit into
w3c:mainfrom
fabiorocha:adding-protocolhandlers
Closed

Add protocol handlers to the explainer and spec#863
fabiorocha wants to merge 1 commit into
w3c:mainfrom
fabiorocha:adding-protocolhandlers

Conversation

@fabiorocha

@fabiorocha fabiorocha commented May 4, 2020

Copy link
Copy Markdown
Member

Closes #846

This change (choose one):

  • Breaks existing normative behavior (please add label "breaking")
  • Adds new normative requirements
  • Adds new normative recommendations or optional items
  • Makes only editorial changes (only changes informative sections, or
    changes normative sections without changing behavior)
  • Is a "chore" (metadata, formatting, fixing warnings, etc).

Implementation commitment (delete if not making normative changes):

Commit message:
Add protocol handlers to the explainer and spec.


Preview | Diff

@fabiorocha

Copy link
Copy Markdown
Member Author

@mgiuca / @marcoscaceres / @aarongustafson this is a first stab at having protocol_handlers added to the spec, as we discussed. Hopefully this can get some discussion around issue #846 going and we can settle on something soon, although I think most people we've talked to feel comfortable with the current proposal. Thanks for all your input so far.

@reillyeon

Copy link
Copy Markdown
Member

@dmurph who registered concerns with the overlap with registerProtocolHandler() on MicrosoftEdge/MSEdgeExplainers#286.

@dmurph

dmurph commented May 4, 2020

Copy link
Copy Markdown
Collaborator

open issues: https://github.com/MicrosoftEdge/MSEdgeExplainers/labels/URLProtocolHandler

@fabiorocha

Copy link
Copy Markdown
Member Author

Thanks Daniel. There are at least a couple that can be moved forward with a PR against the explainer. I'll work on updating those.

@aarongustafson aarongustafson marked this pull request as draft March 8, 2021 20:20
@aarongustafson

Copy link
Copy Markdown
Collaborator

Converted this to a draft while @diekus works on the updates.

@marcoscaceres

Copy link
Copy Markdown
Member

Just a heads up that the spec no longer uses WebIDL and I've changed slightly how processing of members happen. I also got rid of "issue a developer warning", and left that up to UAs.

@marcoscaceres marcoscaceres left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some issues, but a good base to build on! Thanks for putting this together @fabiorocha!

Comment thread index.html
Comment on lines +2098 to +2107
<p class="note">
Protocol handlers could, for instance, be used for web app
communication where one app directly invokes another and passes data
via custom protocol links.
</p>
<p>
How protocol handlers are presented, and how many of them are shown
to the user, is at the discretion of the user agent and/or operating
system.
</p>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't sound amazing from a security perspective. I'm assuming all cross app communication is user mediated?

Comment thread index.html
Comment on lines +2134 to +2136
<li>If <var>protocol_handler</var>["url"] is not <a>within
scope</a> of <var>manifest URL</var>, <a>issue a developer
warning</a> and [=iteration/continue=].

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have a nice shorthand for this:

Suggested change
<li>If <var>protocol_handler</var>["url"] is not <a>within
scope</a> of <var>manifest URL</var>, <a>issue a developer
warning</a> and [=iteration/continue=].
<li>If <var>protocol_handler</var>["url"] is not [=manifest/within
scope=], [=iteration/continue=].

Comment thread index.html
"WEBIDL#sequence-type">sequence</a>&lt;<a>ProtocolHandlerItem</a>&gt;.
</p>
<ol>
<li>Let <var>processedProtocolHandlers</var> be a new Array object

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably make this a set, and then you can later check for duplicates with [=list/contains=].

Comment thread index.html
</p>
<p>
The <a>protocol</a> member of a <a>ProtocolHandlerItem</a> is
equivalent to <code>registerProtocolHandler</code>'s

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
equivalent to <code>registerProtocolHandler</code>'s
equivalent to {{NavigatorContentUtils/registerProtocolHandler()}}'s

Comment thread index.html
Comment on lines +2861 to +2862
<code>scheme</code> argument defined in [[HTML]], and is processed in
the same manner.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably not sufficient... here we want to make sure the HTML folks export something out from there spec that we can call into to process these.

Comment thread index.html
<var>protocol_handler</var>["url"] are undefined, <a>issue a
developer warning</a> and [=iteration/continue=].
</li>
<li>Set <var>protocol_handler</var>["url"] to the result of [=URL

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's best to create a new variable here to store the newly parsed URL.

Comment thread index.html
</h3>
<p>
The <dfn>url</dfn> member of a <a>ProtocolHandlerItem</a> is the
<a>URL</a> <a data-lt="within scope of a manifest">within the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use "manifest scope" here, otherwise it might get confusing as "application scope" is not a defined thing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to "manifest scope".

Comment thread index.html
activated.
</p>
<p>
The <a>url</a> member of a <a>ProtocolHandlerItem</a> is equivalent

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above... this is probably not ideal (as per the TAG comments about layering). We own our own definitions of these things, and then we make them work with the underlying model of HTML.

Comment thread index.html
following steps:
</p>
<ol>
<li>Let <var>url</var> be <var>protocol_handler.url</var>.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The algorithm should operate on the document's "processed manifest" instead.

Comment thread index.html
<ol>
<li>Let <var>url</var> be <var>protocol_handler.url</var>.
</li>
<li>Replace the first occurrence of the exact literal string "%s" in

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this bit probably needs really precise processing as a simple replace here might end up with security issues. If anything, we want to do whatever HTML does ... or maybe whatever Web Share Target does.

cc @mgiuca

@christianliebel

Copy link
Copy Markdown
Member

It looks like protocol handlers are already defined at the Manifest Incubations spec. So, I would go ahead and close this four-year-old draft PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add manifest option for PWAs to be registered as scheme/protocol handlers

7 participants