Add canShare() method #177

marcoscaceres · 2020-09-18T11:23:42Z

Closes #108
Closes #159

For normative changes, the following tasks have been completed:

Modified Web platform tests

Implementation commitment:

WebKit - Safari 14
Chromium - 66?
Gecko

Preview | Diff

marcoscaceres · 2020-09-21T09:40:30Z

Filed Gecko bug and implemented it. Will send patch to Mozilla.

marcoscaceres · 2020-09-21T09:41:00Z

Working on updating the tests also.

index.html

marcoscaceres · 2020-09-23T10:02:46Z

@scheib, I added some rationale text for canShare for #159 ... would you mind taking a look and let me know what you think?

Spec: https://w3c.github.io/web-share/#share-method If a share() request is active when share() is called again, the new share request fails immediately. w3c/web-share#113 In content shell, the OS-integration for the share service is not present. Instead of crashing in tests, we report a not implemented error. WPT web-share/share-sharePromise-internal-slot.https.html now passes (It previously crashed.) Protocols other that http and https are no longer supported by share() or canShare(). w3c/web-share#174 canShare is being discussed in w3c/web-share#177 Bug: 1002337, 1002514, 1131755 Change-Id: I4ec9f6eb03373fd5c6db1881df906a8df36ca4ff

Spec: https://w3c.github.io/web-share/#share-method If a share() request is active when share() is called again, the new share request fails immediately. w3c/web-share#113 In content shell, the OS-integration for the share service is not present. Instead of crashing in tests, we report a not implemented error. WPT web-share/share-sharePromise-internal-slot.https.html now passes - it previously crashed. w3c/web-share#183 improves consistency between the test and the spec. Protocols other that http and https are no longer supported by share() or canShare(). w3c/web-share#174 canShare is being discussed in w3c/web-share#177 Bug: 1002337, 1002514, 1131755 Change-Id: I4ec9f6eb03373fd5c6db1881df906a8df36ca4ff

We now follow the recent spec change limiting the permitted scheme for shared urls to http and https - see w3c/web-share#173 w3c/web-share#174 w3c/web-share#177 We make an exception if the page performing the share it itself loaded from a different scheme (e.g. file) - in that case we allow the same scheme to be used for the shared url. Bug: 1131755 Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e

We now follow the recent spec change limiting the permitted scheme for shared urls to http and https - see w3c/web-share#173 w3c/web-share#174 w3c/web-share#177 We make an exception if the page performing the share it itself loaded from a different scheme (e.g. file) - in that case we allow the same scheme to be used for the shared url. Bug: 1131755 Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977 Commit-Queue: Eric Willigers <ericwilligers@chromium.org> Reviewed-by: Glen Robertson <glenrob@chromium.org> Auto-Submit: Eric Willigers <ericwilligers@chromium.org> Cr-Commit-Position: refs/heads/master@{#810180}

scheib · 2020-09-25T21:47:13Z

Thank you - this addresses #159 well. Text explaining why to use canShare looks good to me.

…nd https, a=testonly Automatic update from web-platform-tests Web Share: restrict URL scheme to http and https We now follow the recent spec change limiting the permitted scheme for shared urls to http and https - see w3c/web-share#173 w3c/web-share#174 w3c/web-share#177 We make an exception if the page performing the share it itself loaded from a different scheme (e.g. file) - in that case we allow the same scheme to be used for the shared url. Bug: 1131755 Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977 Commit-Queue: Eric Willigers <ericwilligers@chromium.org> Reviewed-by: Glen Robertson <glenrob@chromium.org> Auto-Submit: Eric Willigers <ericwilligers@chromium.org> Cr-Commit-Position: refs/heads/master@{#810180} -- wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6 wpt-pr: 25755

…nd https, a=testonly Automatic update from web-platform-tests Web Share: restrict URL scheme to http and https We now follow the recent spec change limiting the permitted scheme for shared urls to http and https - see w3c/web-share#173 w3c/web-share#174 w3c/web-share#177 We make an exception if the page performing the share it itself loaded from a different scheme (e.g. file) - in that case we allow the same scheme to be used for the shared url. Bug: 1131755 Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977 Commit-Queue: Eric Willigers <ericwilligerschromium.org> Reviewed-by: Glen Robertson <glenrobchromium.org> Auto-Submit: Eric Willigers <ericwilligerschromium.org> Cr-Commit-Position: refs/heads/master{#810180} -- wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6 wpt-pr: 25755 UltraBlame original commit: 527027b783378db7e3c1fcdd908e718799400989

…nd https, a=testonly Automatic update from web-platform-tests Web Share: restrict URL scheme to http and https We now follow the recent spec change limiting the permitted scheme for shared urls to http and https - see w3c/web-share#173 w3c/web-share#174 w3c/web-share#177 We make an exception if the page performing the share it itself loaded from a different scheme (e.g. file) - in that case we allow the same scheme to be used for the shared url. Bug: 1131755 Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977 Commit-Queue: Eric Willigers <ericwilligers@chromium.org> Reviewed-by: Glen Robertson <glenrob@chromium.org> Auto-Submit: Eric Willigers <ericwilligers@chromium.org> Cr-Commit-Position: refs/heads/master@{#810180} -- wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6 wpt-pr: 25755

marcoscaceres · 2021-08-04T03:45:03Z

Pinging again 👋 I'd like get this merge so we can move the spec to CR.

saschanaz · 2021-08-07T20:24:13Z

index.html

+          The choice to use string literals instead of an [=enumeration=] is
+          deliberate. Doing so allows implementers to gracefully support future
+          {{ShareData}} member values, while also giving the user agent the
+          ability to disable sharing those members if needed.


Should this mention whatwg/webidl#893 or whatwg/webidl#491?

No. If something changes in WebIDL, then it should be adopted. whatwg/webidl#491 should be closed anyway - it's no longer relevant.

saschanaz · 2021-08-07T20:25:50Z

How can we get diff preview as other w3c repositories have?

saschanaz · 2021-08-09T01:38:44Z

index.html

@@ -270,6 +268,110 @@ <h4>
            or bypassing the UI if there is only a single share target.
          </div>
        </section>
+        <section>
+          <h3>
+            `canShare(data)` method


Should this be marked as historical if we think this is a broken approach?

Only if we are not going to ship an alternative - but I don't think anyone plans to.

Only if we are not going to ship an alternative

Are going to?

Oops, yes... there is no plans for any engine to ship anything different. Let's just ship this and be done for now. If we need a new method, we can add it when there is demand for it.

I don't think Gecko would want to ship anything proven to be broken as a sole solution 🤔

cc @annevk, context: #108 (comment)

That's ok, but if we can't shift the other implementers then we need to make a process call here (i.e., Mozilla can "formally object" and we figure it out from there).

We have two engines that already ship .canShare() already (and have now for over a year).

I think it depends on real world usage of canShare.

You are right, that too... however, we (you and I) are kinda limited in that we can't actually get that data.

We are at an impasse here :( without input from other implementers willing to change (or even respond to issues), I don't know what we can do 😭

You are right, that too... however, we (you and I) are kinda limited in that we can't actually get that data.

https://chromestatus.com/metrics/feature/timeline/popularity/2737

marcoscaceres · 2021-08-09T01:39:31Z

How can we get diff preview as other w3c repositories have?

We might not have had .pr_preview enabled when the PR was created a long time ago... I'll see if I can get it working.

marcoscaceres · 2021-08-09T01:48:31Z

PR preview is back 🎉

marcoscaceres · 2021-08-18T07:12:50Z

Ok, sent a PR to remove "tentative" from the canShare() tests. The tests all pass in WebKit, except 1 that I need to look into.

marcoscaceres · 2021-08-20T01:48:41Z

I'm going ahead and merging this as this matches two shipping implementations, and this is been sitting for nearly a year. I'm happy to continue to revise canShare() and even send patches to various engines if we want to improve it in various ways - so long as things remain backwards compatible.

I'll also make another attempt to update the Gecko patch.

We now follow the recent spec change limiting the permitted scheme for shared urls to http and https - see w3c/web-share#173 w3c/web-share#174 w3c/web-share#177 We make an exception if the page performing the share it itself loaded from a different scheme (e.g. file) - in that case we allow the same scheme to be used for the shared url. Bug: 1131755 Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977 Commit-Queue: Eric Willigers <ericwilligers@chromium.org> Reviewed-by: Glen Robertson <glenrob@chromium.org> Auto-Submit: Eric Willigers <ericwilligers@chromium.org> Cr-Commit-Position: refs/heads/master@{#810180} GitOrigin-RevId: 060b7f1b2de01048a934bc4aca41973edaf4d12c

marcoscaceres force-pushed the canShare branch from 0f2ed2c to 99cccb7 Compare September 21, 2020 09:34

marcoscaceres marked this pull request as ready for review September 21, 2020 09:39

marcoscaceres mentioned this pull request Sep 22, 2020

Limiting to http/https is limiting #178

Closed

marcoscaceres requested a review from ewilligers September 22, 2020 10:42

saschanaz reviewed Sep 22, 2020

View reviewed changes

index.html Outdated Show resolved Hide resolved

marcoscaceres mentioned this pull request Sep 23, 2020

chore: inform people that l2 is folded into main spec #134

Merged

chromium-wpt-export-bot mentioned this pull request Sep 24, 2020

Web Share: Only allow one share() call at a time web-platform-tests/wpt#25747

Closed

chromium-wpt-export-bot mentioned this pull request Sep 24, 2020

Web Share: restrict URL scheme to http and https web-platform-tests/wpt#25755

Merged

This was referenced Sep 25, 2020

Explain why canShare exists at definition. #159

Closed

Introduce canShareFiles #184

Closed

marcoscaceres added 4 commits October 28, 2020 16:05

Add canShare() method

0331a1c

add SecureContext and allowed to use check

a130e8d

Add http+https checks to validation

f36224b

Handle empty files sequence

0ec77e9

marcoscaceres added 2 commits June 17, 2021 13:31

Merge branch 'main' into canShare

f38862e

Merge branch 'main' into canShare

d4853d8

w3c deleted a comment Aug 7, 2021

saschanaz reviewed Aug 7, 2021

View reviewed changes

marcoscaceres mentioned this pull request Aug 8, 2021

Plan to transition to CR #207

Closed

marcoscaceres added 2 commits August 9, 2021 09:13

Merge branch 'main' into canShare

6c04cb2

Merge branch 'main' into canShare

7828d97

saschanaz reviewed Aug 9, 2021

View reviewed changes

marcoscaceres closed this Aug 9, 2021

marcoscaceres reopened this Aug 9, 2021

Merge branch 'main' into canShare

07322d5

Merge branch 'main' into canShare

5dc2244

marcoscaceres mentioned this pull request Aug 13, 2021

canShare() is not future compatible #108

Open

marcoscaceres added 3 commits August 16, 2021 17:43

Merge branch 'main' into canShare

7875616

Merge branch 'main' into canShare

cca9754

Merge branch 'main' into canShare

cad9d6e

marcoscaceres mentioned this pull request Aug 17, 2021

Add "validate share data" algorithm #185

Merged

4 tasks

marcoscaceres added 2 commits August 18, 2021 15:35

Merge branch 'main' into canShare

b3ef905

Specify minimum canShare() to match Chrome/Safari

338d7fa

marcoscaceres mentioned this pull request Aug 18, 2021

Make canShare() tests no longer tentative web-platform-tests/wpt#30066

Merged

marcoscaceres merged commit 68494b7 into main Aug 20, 2021

marcoscaceres deleted the canShare branch August 20, 2021 01:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add canShare() method #177

Add canShare() method #177

marcoscaceres commented Sep 18, 2020 •

edited by pr-preview bot

Loading

marcoscaceres commented Sep 21, 2020

marcoscaceres commented Sep 21, 2020

marcoscaceres commented Sep 23, 2020

scheib commented Sep 25, 2020

marcoscaceres commented Aug 4, 2021

saschanaz Aug 7, 2021

marcoscaceres Aug 8, 2021

saschanaz commented Aug 7, 2021

saschanaz Aug 9, 2021

marcoscaceres Aug 9, 2021

saschanaz Aug 9, 2021

marcoscaceres Aug 9, 2021

saschanaz Aug 9, 2021

marcoscaceres Aug 9, 2021

saschanaz Aug 9, 2021

marcoscaceres Aug 13, 2021

ewilligers Aug 23, 2021

marcoscaceres commented Aug 9, 2021

marcoscaceres commented Aug 9, 2021

marcoscaceres commented Aug 18, 2021

marcoscaceres commented Aug 20, 2021

Add canShare() method #177

Add canShare() method #177

Conversation

marcoscaceres commented Sep 18, 2020 • edited by pr-preview bot Loading

marcoscaceres commented Sep 21, 2020

marcoscaceres commented Sep 21, 2020

marcoscaceres commented Sep 23, 2020

scheib commented Sep 25, 2020

marcoscaceres commented Aug 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saschanaz commented Aug 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoscaceres commented Aug 9, 2021

marcoscaceres commented Aug 9, 2021

marcoscaceres commented Aug 18, 2021

marcoscaceres commented Aug 20, 2021

marcoscaceres commented Sep 18, 2020 •

edited by pr-preview bot

Loading