Add `unsanitized` option to async clipboard API. #197

snianu · 2023-10-30T22:13:56Z

Closes #????

For normative changes, the following tasks have been completed:

Modified Web platform tests (link to pull request) [Clipboard API] Read unsanitized HTML content to the clipboard web-platform-tests/wpt#35528

Implementation commitment:

WebKit (https://bugs.webkit.org/show_bug.cgi?id=)
Chromium (https://bugs.chromium.org/p/chromium/issues/detail?id=1268679)
Gecko (https://bugzilla.mozilla.org/show_bug.cgi?id=1867331)

Preview | Diff

snianu · 2023-10-30T22:14:55Z

@evanstade Made spec changes for unsanitized option. Please let me know if you have any comments/concerns. Thanks!

yisibl · 2023-10-31T08:02:05Z

I would like the unsanitized option to be applied to image/svg+xml as well, which would solve the problem of losing <style> when copying SVG. See https://bugs.chromium.org/p/chromium/issues/detail?id=1226150

index.bs

snianu · 2023-10-31T19:33:26Z

Tagging @EdgarChen in this PR as well. Could you please take a look if you are OK with this change? Thanks!

index.bs

snianu · 2023-10-31T21:31:11Z

#197 (comment)

Clarified in the latest iteration.

index.bs

evanstade · 2023-11-03T16:29:18Z

index.bs

+
+			1. If |formats| is not empty, then:
+
+					1. If |formats|'s size is greater than 1, then [=reject=] |p| with {{"NotAllowedError"}} {{DOMException}} in |realm|.


Feels odd to me that the IDL is written to support more than one unsanitized format, but the algorithm wording is only structured to support exactly one sanitized format. Why not iterate over the whole list and reject if any of them are not in the allowed-to-be-sanitized list?

Also if more formats were added in the future, the set of permittable formats could vary by browser. How are sites to know which formats they can ask for? Is throwing an error the best way to deal with them asking for the wrong formats to be unsanitized, or should some format values just be ignored? If throwing an error, how does the site figure out which format(s) were in error?

I don't have a ton of experience with spec language/algos. Can you find a spec mentor to help with these questions?

Addressed these concerns in the latest iteration. Please let me know if I missed anything. Thanks!

evanstade · 2023-11-03T16:35:00Z

index.bs

@@ -841,7 +861,11 @@ url: https://w3c.github.io/permissions/#permissions-task-source; type: dfn;

 							Issue: It should be possible to read the data asynchronously from the system clipboard after the author calls getType, however, this set of steps implies that data will be provided at the time of read.

-						1. The user agent, MAY sanitize |representation|'s [=representation/data=], unless |representation|'s [=representation/MIME type=]'s essence is "image/png", which should remain unsanitized to preserve meta data.
+						1. The user agent, MAY sanitize |representation|'s [=representation/data=], unless it satisfies the below conditions:


It seems like "sanitize" should be defined.

Yeah, I agree with you, but we spent almost a year and half trying to define sanitization, and neither Gecko nor Webkit wants to agree on a definition. At this point I think we just want to add unsanitized option to this spec and leave the contentious algorithms out of the spec.

We don't have to necessarily define the exact steps that constitute hyopthetical sanitization, but it does seem that it would be useful to describe what it's not, because another way to word this text is: "The user agent MUST NOT sanitize the data if it satisfies the below conditions". So what exactly is the user agent not allowed to do, or must it do, if it's providing "unsanitized" data?

In particular, this came up in code review where, after some discussion, it was decided that the CFHTML fragment tags should be left in place. The spec should be clear about this so other vendors don't come to a different conclusion at the implementation step.

index.bs

snianu · 2023-11-20T18:22:30Z

@evanstade I think the latest iteration addresses all your concerns, but please let me know if there is anything missing. I would like to restart the I2S process as I'm not receiving any response from Mozilla folks.

snianu · 2023-11-21T02:54:41Z

Landing this PR. Please let me know if you have any comments and I'll address it in separate PRs.

SHA: c1d37ae Reason: push, by snianu Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

SHA: c1d37ae Reason: push, by pull[bot] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

zcorpan · 2023-11-29T12:56:41Z

It looks like some of the checkboxes in the issue template weren't completed before merging. Was that intentional?

snianu · 2023-11-29T17:56:29Z

@zcorpan Sorry I created a new bug in Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1867331
Here is the request for position: https://chromestatus.com/feature/5192271976988672?gate=5118722541092864
For Webkit they weren't interested in this feature as their sanitization process in async clipboard APIs aligns with the DataTransfer APIs.

saschanaz · 2023-12-02T20:52:19Z

"Implementation commitment" doesn't sound like just filing bugs but more about implementation interest 🤔, in that sense I'm not sure the checkboxes are correct.

snianu · 2023-12-04T17:33:43Z

@saschanaz We have also filed a request for position: mozilla/standards-positions#769

saschanaz · 2023-12-04T18:04:38Z

That's nice, but my understanding of commitment is that there at least should be some interest. Having a bug does not mean an interest, right?

snianu · 2023-12-04T18:51:54Z

@saschanaz right and in that case the bug can be resolved as "Won't fix" with a comment that the user agent is not interested in this feature, and the request for position should be marked accordingly.

saschanaz · 2023-12-04T19:02:16Z

Sure. What I feel off here is the checkbox for Gecko is filled, when there's no public implementation commitment from Gecko.

Edit: ... which means there should be only one filled checkbox for commitment, which should have blocked this PR from being merged I guess.

snianu · 2023-12-04T19:55:49Z

@saschanaz We can remove it from the spec if we only have one implementor interest, but waiting indefinitely for a signal is probably not the right way to decide whether there is interest from a particular browser. If the official stance of Firefox is negative, then please let me know and I can revert this PR and create another PR and mark it as BLOCKED. That way if Firefox or Webkit changes their stance and decide to support this feature in the future, we can unblock the PR and merge it.

saschanaz · 2023-12-04T20:11:56Z

I see your point, but then how are we going to track which part doesn't have implementer consensus and which part does? Having an open PR is easier for that and doing so automatically prevents any part of the spec without consensus from accidentally becoming CR.

snianu · 2023-12-04T23:16:07Z

Note that the IDL and the language in the algorithms already allow non-supporting implementors to be in compliant with the spec while also allowing implementors who are interested in the new feature. I don't see any parts in the PR that is contentious unless I'm missing something?

saschanaz · 2023-12-05T13:42:55Z

I'm talking about the general process rather than the PR itself. And also I don't think we'll want to add any kind of optional features without consensus, just because it's okay to not implement it.

snianu · 2023-12-05T17:55:49Z

And also I don't think we'll want to add any kind of optional features without consensus

We spent over two years trying to get consensus on this API, but we couldn't. Safari has clearly stated their position, so since you commented that you are not in favor of an optional parameter, should I mark Firefox's position as negative?

saschanaz · 2023-12-05T18:12:51Z

Our position will be announced in our standards-position issue probably by Edgar. I definitely see your frustration, but still I think merging PRs without consensus makes it hard to make sure every part of the spec has consensus (which is needed for recommendation track), and I see no benefits to merge it early.

snianu · 2023-12-05T18:31:58Z

@saschanaz Let's discuss this in the EditingWG meeting which is going to happen next Thursday. Without any clear stance on this API, it's hard to assume whether we have consensus or not. If you can officially state your position before that, we can definitely revert the PR, else, we will have to decide what to do with it in the EditingWG meeting where hopefully someone from Firefox can give us clear official stance on this issue. Thanks!

Add unsanitized option to async clipboard API.

dcf28f6

snianu assigned sanketj Oct 30, 2023

yisibl mentioned this pull request Oct 31, 2023

Make it possible to read image/svg+xml from the clipboard #92

Closed

sanketj requested changes Oct 31, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

Address PR comments.

e8c1a9b

sanketj requested changes Oct 31, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

Address PR comments.

26100d7

sanketj requested changes Nov 2, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

sanketj requested a review from EdgarChen November 2, 2023 05:25

evanstade reviewed Nov 3, 2023

View reviewed changes

Address PR comments.

ddb5503

sanketj requested changes Nov 14, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

Addressed PR comments.

9da00db

sanketj reviewed Nov 15, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Addressed PR comments.

ad4077b

sanketj requested changes Nov 16, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Show resolved Hide resolved

Addressed comments.

4211ab2

sanketj requested changes Nov 17, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

evanstade mentioned this pull request Nov 17, 2023

Read Blob data for the supported formats on-demand during getType. #191

Open

snianu added 3 commits November 17, 2023 17:04

Address comments.

f96a21c

Address comments.

9e5e1b7

Add clarification.

0312e3e

sanketj approved these changes Nov 20, 2023

View reviewed changes

snianu merged commit c1d37ae into main Nov 21, 2023
2 checks passed

snianu deleted the read-unsanitized branch November 21, 2023 02:54

github-actions bot added a commit that referenced this pull request Nov 21, 2023

Add unsanitized option to async clipboard API. (#197)

c985fd4

SHA: c1d37ae Reason: push, by snianu Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

snianu added the Agenda+ label Dec 5, 2023

saschanaz mentioned this pull request Dec 14, 2023

Merge a PR only with explicit implementor consensus (followup) #207

Open

EdgarChen mentioned this pull request Dec 27, 2023

Add non-normative and optional texts. #208

Merged

4 tasks

sanketj removed the Agenda+ label Jun 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `unsanitized` option to async clipboard API. #197

Add `unsanitized` option to async clipboard API. #197

snianu commented Oct 30, 2023 •

edited

Loading

snianu commented Oct 30, 2023

yisibl commented Oct 31, 2023

snianu commented Oct 31, 2023

snianu commented Oct 31, 2023

evanstade Nov 3, 2023

snianu Nov 10, 2023

evanstade Nov 3, 2023

snianu Nov 3, 2023

evanstade Nov 3, 2023

snianu commented Nov 20, 2023

snianu commented Nov 21, 2023

zcorpan commented Nov 29, 2023

snianu commented Nov 29, 2023

saschanaz commented Dec 2, 2023 •

edited

Loading

snianu commented Dec 4, 2023

saschanaz commented Dec 4, 2023

snianu commented Dec 4, 2023

saschanaz commented Dec 4, 2023 •

edited

Loading

snianu commented Dec 4, 2023

saschanaz commented Dec 4, 2023 •

edited

Loading

snianu commented Dec 4, 2023

saschanaz commented Dec 5, 2023 •

edited

Loading

snianu commented Dec 5, 2023

saschanaz commented Dec 5, 2023

snianu commented Dec 5, 2023


		1. If \|formats\| is not empty, then:

		1. If \|formats\|'s size is greater than 1, then [=reject=] \|p\| with {{"NotAllowedError"}} {{DOMException}} in \|realm\|.

Add unsanitized option to async clipboard API. #197

Add unsanitized option to async clipboard API. #197

Conversation

snianu commented Oct 30, 2023 • edited Loading

snianu commented Oct 30, 2023

yisibl commented Oct 31, 2023

snianu commented Oct 31, 2023

snianu commented Oct 31, 2023

evanstade Nov 3, 2023

Choose a reason for hiding this comment

snianu Nov 10, 2023

Choose a reason for hiding this comment

evanstade Nov 3, 2023

Choose a reason for hiding this comment

snianu Nov 3, 2023

Choose a reason for hiding this comment

evanstade Nov 3, 2023

Choose a reason for hiding this comment

snianu commented Nov 20, 2023

snianu commented Nov 21, 2023

zcorpan commented Nov 29, 2023

snianu commented Nov 29, 2023

saschanaz commented Dec 2, 2023 • edited Loading

snianu commented Dec 4, 2023

saschanaz commented Dec 4, 2023

snianu commented Dec 4, 2023

saschanaz commented Dec 4, 2023 • edited Loading

snianu commented Dec 4, 2023

saschanaz commented Dec 4, 2023 • edited Loading

snianu commented Dec 4, 2023

saschanaz commented Dec 5, 2023 • edited Loading

snianu commented Dec 5, 2023

saschanaz commented Dec 5, 2023

snianu commented Dec 5, 2023

Add `unsanitized` option to async clipboard API. #197

Add `unsanitized` option to async clipboard API. #197

snianu commented Oct 30, 2023 •

edited

Loading

saschanaz commented Dec 2, 2023 •

edited

Loading

saschanaz commented Dec 4, 2023 •

edited

Loading

saschanaz commented Dec 4, 2023 •

edited

Loading

saschanaz commented Dec 5, 2023 •

edited

Loading