-
Notifications
You must be signed in to change notification settings - Fork 66
Introduce [[sharePromise]] internal slot #113
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
Conversation
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 good. Thanks.
Oops forgot: You need to add these changes to Level 2 as well. L2 isn't a patch on L1, it's a "fork". Or, just land this and I'll copy-paste it to L2 if you don't have the time (let me know). |
9ae677a
to
0076738
Compare
Blocking myself on providing tests. |
If you don't mind, please update the L2 for me :) |
OK, after you're ready to merge (or after you merge), so that we don't have to keep syncing. (Unfortunately, pr-preview didn't work because it only looks at level-2. I can't find a way for pr-preview to look at more than one document.) |
Yeah, unfortunately, I don't think that's supported yet. @tobie can probably confirm? |
On Gecko side, part of https://bugzilla.mozilla.org/show_bug.cgi?id=1312422 |
@mgiuca, will you handle filing bugs on the Chromium side? |
Yeah, that's not supported for now. I'd be open to build the feature is someone was interested in sponsoring the work. |
@mgiuca or @ericwilligers, could you please review the associated test? web-platform-tests/wpt#18486 |
0076738
to
f2a3126
Compare
@ericwilligers or @mgiuca, if you have a minute, could you please review web-platform-tests/wpt#18486? |
Filed Chrome bug https://bugs.chromium.org/p/chromium/issues/detail?id=1002337 |
Filed WebKit bug https://bugs.webkit.org/show_bug.cgi?id=201627 |
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 update behavior to match spec change w3c/web-share#113 If a share() call is in progress and share() is called again, the new share request fails immediately. WebShare for ChromeOS and Windows will only ever have this new behavior. WebShare for Android (already shipped) would previously spin up multiple shares simultaneously. Spec: https://w3c.github.io/web-share/#share-method Note: we no longer have an Android post Lollipop MR1 test for user cancellation of the dialog - the existing test simply issues a second share request. Bug: 1002337 Change-Id: I1bef4923cb048a28f882f9dac889c841f1fc8265 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425933 Reviewed-by: Ted Choc <[email protected]> Commit-Queue: Eric Willigers <[email protected]> Cr-Commit-Position: refs/heads/master@{#811265}
While the promise has not yet resolved or rejected, we disable the share button to prevent additional share attempts. This avoids confusion arising from [[sharePromise]] internal slot (w3c#113)
While the promise has not yet resolved or rejected, we disable the share button to prevent additional share attempts. This avoids confusion arising from [[sharePromise]] internal slot (#113)
Fixed also in WebKit from yesterday https://bugs.webkit.org/show_bug.cgi?id=201627 |
For normative changes, the following tasks have been completed:
Implementation commitment:
For discussion... this prevents #110 (multiple concurrent shares).