Skip to content

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

Merged
merged 1 commit into from
Sep 10, 2019
Merged

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Jul 8, 2019

For normative changes, the following tasks have been completed:

Implementation commitment:

For discussion... this prevents #110 (multiple concurrent shares).

@marcoscaceres marcoscaceres requested a review from ewilligers July 8, 2019 02:12
@marcoscaceres marcoscaceres requested a review from mgiuca August 7, 2019 05:09
@marcoscaceres marcoscaceres marked this pull request as ready for review August 15, 2019 04:22
Copy link
Collaborator

@mgiuca mgiuca left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

@mgiuca
Copy link
Collaborator

mgiuca commented Aug 16, 2019

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).

@marcoscaceres
Copy link
Member Author

Blocking myself on providing tests.

@marcoscaceres
Copy link
Member Author

If you don't mind, please update the L2 for me :)

@mgiuca
Copy link
Collaborator

mgiuca commented Aug 16, 2019

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.)

@marcoscaceres
Copy link
Member Author

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?

@marcoscaceres
Copy link
Member Author

On Gecko side, part of https://bugzilla.mozilla.org/show_bug.cgi?id=1312422

@marcoscaceres
Copy link
Member Author

@mgiuca, will you handle filing bugs on the Chromium side?

@tobie
Copy link
Member

tobie commented Aug 17, 2019

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?

Yeah, that's not supported for now.

I'd be open to build the feature is someone was interested in sponsoring the work.

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Sep 6, 2019

@mgiuca or @ericwilligers, could you please review the associated test? web-platform-tests/wpt#18486

@marcoscaceres
Copy link
Member Author

@ericwilligers or @mgiuca, if you have a minute, could you please review web-platform-tests/wpt#18486?

@marcoscaceres
Copy link
Member Author

@marcoscaceres
Copy link
Member Author

Filed WebKit bug https://bugs.webkit.org/show_bug.cgi?id=201627

@marcoscaceres marcoscaceres merged commit 0c8e0c1 into master Sep 10, 2019
@marcoscaceres marcoscaceres deleted the sharePromise branch September 16, 2019 05:21
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 24, 2020
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
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 24, 2020
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
pull bot pushed a commit to Mu-L/chromium that referenced this pull request Sep 28, 2020
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}
ewilligers pushed a commit to ewilligers/web-share that referenced this pull request Jan 12, 2021
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)
ewilligers added a commit that referenced this pull request Jan 20, 2021
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)
@marcoscaceres
Copy link
Member Author

Fixed also in WebKit from yesterday https://bugs.webkit.org/show_bug.cgi?id=201627

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.

Distinguishing between data errors and user abort Clarifying behavior of multiple simultaneous shares
4 participants