Skip to content
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

Remove the title argument from registerProtocolHandler() #5425

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

annevk
Copy link
Member

@annevk annevk commented Apr 1, 2020

No implementation does something with it UI-wise, fortunately. Unfortunately it's still observable, so this would require implementation changes and a test to see that it's in fact ignored and not stringified.

cc @ericlaw1979 @fred-wang

(See WHATWG Working Mode: Changes for more details.)


/system-state.html ( diff )

@annevk annevk added the removal/deprecation Removing or deprecating a feature label Apr 1, 2020
annevk added a commit to web-platform-tests/wpt that referenced this pull request Apr 3, 2020
@domenic domenic added the needs implementer interest Moving the issue forward requires implementers to express interest label Apr 3, 2020
@domenic
Copy link
Member

domenic commented Apr 3, 2020

@mgiuca, are you still working in this area of Chrome, or know someone who is? This is a pretty small change.

@connor-moody
Copy link

Related to this change, we at Microsoft are currently working on adding a standard for PWA protocol handlers with an associated title field (explainer here).

We envision that the title field will be useful for larger apps with multiple functions (e.g. a mail app with an email handler and a calendar handler or a messaging app that handles irc:// and sms://). At an implementation level, we plan on making the title visible both in the browser and in the OS (where possible) to make PWAs feel more like native apps to users.

While PWA protocol handlers and registerProtocolHandler() will not strictly be tied together in the web standard, including a title argument in both would provide a more unified UX between the two.

@mgiuca
Copy link
Contributor

mgiuca commented Apr 5, 2020

Happy to let Connor think about this in detail (their team has been working in this area of Chromium).

My high-level guidance has been to try and keep the programmatic registerProtocolHandler and the newly proposed declarative protocol_handlers as close as possible, so if they think the title is useful for one, it should maybe be kept for both.

Having said that, we do want to be careful about spoofing when we let apps set custom titles for things that aren't their main title (e.g. you install an app called "Monkey Game" and suddenly it's showing protocol handlers for "Bank of America"). That's the reason why, for Web Share Target, we only allow a single share target, and it has no explicit "title" field (so we always name the share target after the title of the application, which the user agreed to during the install prompt). I feel like the same rationale applies to protocol handlers.

We envision that the title field will be useful for larger apps with multiple functions (e.g. a mail app with an email handler and a calendar handler or a messaging app that handles irc:// and sms://). At an implementation level, we plan on making the title visible both in the browser and in the OS (where possible) to make PWAs feel more like native apps to users.

If you had such an application, say "Microsoft Outlook", would there be any reason to have separate custom titles for the irc and sms protocol handlers? Why would it be insufficient to just show the title of the application itself, "Microsoft Outlook"?

@connor-moody
Copy link

@mgiuca you bring up a valid concern about the security implications of including a title. In the spec for PWA handlers, we would want to include guidance similar to the "Misleading Titles" section of the registerProtocolHandlers standard, pointing out that implementations should always display origin along with the title.

Regarding your question about using the app name in place of custom titles - in the case you referenced, using "Microsoft Outlook" as a handler title may be sufficient. A different case where a custom title field may prove useful is if a user has installed a native app and a PWA of the same name. If user has both the native and PWA versions of "Microsoft Word" installed with no custom title, OS disambigation dialogs would confusingly prompt the user to choose between "Microsoft Word" and "Microsoft Word". If a custom title is available, a user could be prompted to choose between "Microsoft Word Classic" and "Microsoft Word Online".

Whatever the consensus on a title for registerProtocolHandler() ends up being, we hope to align PWA protocol_handlers as closely as possible to improve UX around the APIs.

@mgiuca
Copy link
Contributor

mgiuca commented Apr 8, 2020

Regarding those specific examples: I feel like the PWA vs native app disambiguation case is more general than protocol handlers. If you need to disambiguate the app name in the protocol handler so the user can tell the difference between two otherwise identical apps, then surely you also need to disambiguate in OS shortcuts, share targets, store listings, etc... anywhere the app title is displayed. So again, I feel like this isn't a use case for having a protocol handler with a different title to its app title.

@connor-moody
Copy link

Good point, it seems like a custom title field would add complexity without significant value. We will remove the title field from our protocol_handlers proposal, which means that the removal of the title argument from registerProtocolHandler() can continue as planned.

@fred-wang
Copy link
Contributor

fred-wang commented Apr 20, 2020

So it seems there is consensus to remove the title argument, right? What is missing before merging this PR? On the implementation side, Igalia is happy to send patches / intent-to to Firefox / Chromium.

@annevk Probably the comment aligning PWA's protocol_handlers and registerProtocolHandler, also applies to extensions' protocol_handlers ( https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/protocol_handlers )? I assume we want to remove (or at least deprecate) "name" and rename the keyword (or at least add a new one and deprecate current one) uriTemplate => url?

@annevk
Copy link
Member Author

annevk commented Apr 20, 2020

Maybe, extensions have a different threat model to some extent and are someone outside the realm of web standards.

What's missing is implementation bugs as it does seem like we have buy-in from Chrome/Firefox.

@fred-wang
Copy link
Contributor

Maybe, extensions have a different threat model to some extent and are someone outside the realm of web standards.

OK, I guess "name" is not relying on registerProtocolHandler()'s title support anyway, so it seems fine to drop the latter.

What's missing is implementation bugs as it does seem like we have buy-in from Chrome/Firefox.

I'll open those bugs. Was not sure whether that was needed or if a "lgtm" from Chrome/Firefox was enough here.

@annevk
Copy link
Member Author

annevk commented Apr 20, 2020

Per https://whatwg.org/working-mode#changes we'd like to have explicit bugs on file (linked from here). This helps future readers and ensures everyone has been informed of the change.

@fred-wang
Copy link
Contributor

  * Chrome: …
  * Firefox: …
  * Safari: …

I filed bugs for Chromium and Mozilla:
https://bugs.chromium.org/p/chromium/issues/detail?id=1072461
https://bugzilla.mozilla.org/show_bug.cgi?id=1631464

WebKit does not support registerProtocolHandler() so not sure whether it makes sense to open a bug to remove the title argument.

@annevk annevk removed the needs implementer interest Moving the issue forward requires implementers to express interest label Apr 20, 2020
annevk added a commit to web-platform-tests/wpt that referenced this pull request Apr 20, 2020
@annevk annevk merged commit af228b2 into master Apr 20, 2020
@annevk annevk deleted the annevk/registerProtocolHandler-title-argument branch April 20, 2020 15:55
@annevk
Copy link
Member Author

annevk commented Apr 20, 2020

Thanks all!

fred-wang added a commit to web-platform-tests/wpt that referenced this pull request Apr 24, 2020
This was discussed in [1] and test added in [2]. This commit updates
the corresponding IDL test.

[1] whatwg/html#5425
[2] #22678
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 1, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 1, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 1, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 1, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request May 3, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678

UltraBlame original commit: 24cfed11a1be4dbc4740eb485c39f3ae138641d9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request May 3, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678

UltraBlame original commit: 0cc373eb4538fdb5bdc70153b118a66164067f31
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request May 3, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678

UltraBlame original commit: 24cfed11a1be4dbc4740eb485c39f3ae138641d9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request May 3, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678

UltraBlame original commit: 0cc373eb4538fdb5bdc70153b118a66164067f31
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request May 3, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678

UltraBlame original commit: 24cfed11a1be4dbc4740eb485c39f3ae138641d9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request May 3, 2020
…ProtocolHandler(), a=testonly

Automatic update from web-platform-tests
HTML: remove title argument for registerProtocolHandler()

For whatwg/html#5425.

--

wpt-commits: 5f24b1946045577454f2b93300d7fdbad9921515
wpt-pr: 22678

UltraBlame original commit: 0cc373eb4538fdb5bdc70153b118a66164067f31
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Nov 4, 2020
This CL removes the title argument from registerProtocolHandler() as
agreed in the WHATWG [1]. See also the discussion on the blink-dev
mailing list [2]. The only other browser implementation was in Firefox
but it has been removed [3].

[1] whatwg/html#5425
[2] https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/cZh5f1Ymj14/rLz2Xsy0BAAJ
[3] https://groups.google.com/d/msg/mozilla.dev.platform/T633aemFPJU/dbv5_iB8AgAJ

Bug: 1072461
Change-Id: Ia9f60203939fc756950d511c923a3e9dd2256100
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2157531
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823981}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
removal/deprecation Removing or deprecating a feature
Development

Successfully merging this pull request may close these issues.

5 participants