Defer OS clipboard read from clipboard.read() to ClipboardItem.getType()#248
Defer OS clipboard read from clipboard.read() to ClipboardItem.getType()#248shwetabin wants to merge 8 commits into
Conversation
051ab76 to
5d6fa4f
Compare
- read() now snapshots only format names and clipboard sequence number, setting representation data to unresolved promises - getType() performs the actual OS clipboard read lazily on demand - Stale clipboard check added to getType() (rejects with DataError if clipboard changed since read()) - Sanitization moved from read() to getType() - Added isUnsanitized flag to representation and clipboard sequence number to ClipboardItem Co-Authored-By: Shweta Bindal <shwetabindal@microsoft.com>
5d6fa4f to
4e0ff1b
Compare
|
Can you mention in the commit message that this fixes #240? At least I assume that's the goal. |
|
hi @shwetabin , could you link your GitHub account with your W3C account please? This will help our IPR bot recognize you as a working group member. |
Co-Authored-By: Shweta Bindal <sbindal@microsoft.com>
…s map; define `reading bytes` algorithm on the system clipboard. Addresses Edgar's review on w3c#248.
|
|
||
| 1. Else, let |osFormatName| be the result of running [=os specific well-known format=] given |mimeType|. | ||
|
|
||
| 1. Let |clipboardItem| be the [=system clipboard item=] currently held in the [=system clipboard=]. If there is no such item, then [=queue a global task=] on the [=clipboard task source=], given |realm|'s [=realm/global object=], to [=reject=] |p| with a {{"NotFoundError"}} {{DOMException}} in |realm|, then abort these steps. |
There was a problem hiding this comment.
Shouldn't this still be done while in parallel?
There was a problem hiding this comment.
You're right — the steps from "Let |clipboardItem|…" through the blob construction had drifted out of the in parallel block; re-indented them so only the final queue a global task to resolve/reject crosses back to the event loop.
|
|
||
| 1. Else, let |osFormatName| be the result of running [=os specific well-known format=] given |mimeType|. | ||
|
|
||
| 1. Let |clipboardItem| be the [=system clipboard item=] currently held in the [=system clipboard=]. If there is no such item, then [=queue a global task=] on the [=clipboard task source=], given |realm|'s [=realm/global object=], to [=reject=] |p| with a {{"NotFoundError"}} {{DOMException}} in |realm|, then abort these steps. |
There was a problem hiding this comment.
Also, which item are we talking about here? How are they mapped?
There was a problem hiding this comment.
The mapping is implicit via the [=ClipboardItem/clipboard change count at read=] check earlier in this algorithm: if the [=clipboard change count=] hasn't changed since {{Clipboard/read()}}, the "currently held" [=system clipboard item=] is by construction the same one {{Clipboard/read()}} observed. In practice the [=system clipboard=] holds at most one item at a time, so "currently held" is unambiguous.
There was a problem hiding this comment.
Is that true on all platforms? But if that's the case we should make that clear in the data model. Because if in practice and in theory differ, it's certainly ambiguous.
|
|
||
| 1. Let |realm| be [=this=]'s [=relevant realm=]. | ||
|
|
||
| 1. Let |originalType| be |type|. |
There was a problem hiding this comment.
Is |type| here completely not normalized? How can that reasonably be used as a map key?
There was a problem hiding this comment.
Good catch — switched the map key from the raw |type| to a normalized one (serialize |mimeType|, prefixed with "web " when |isCustom| is true), so differently-cased spellings of the same type now collapse to a single entry.
There was a problem hiding this comment.
I think we probably only want to use mimeType's essence here, not the serialization including all parameters.
…inside in-parallel block Addresses Anne's review on w3c#248: - Replace raw |type| map key with normalized key derived from serialized |mimeType| (+ "web " prefix when |isCustom|), so case-differing spellings of the same type collapse to one entry. - Re-indent the system clipboard fetch / sanitize / blob steps so they remain inside the `in parallel` block; only the final `queue a global task` crosses back to the event loop.
…essence as map key Addresses Anne's follow-up review on w3c#248: - Add `originating system clipboard item` slot on ClipboardItem, set at Clipboard.read() to the specific system clipboard item the ClipboardItem was derived from. ClipboardItem.getType() reads from that slot rather than "the system clipboard item currently held", making the algorithm unambiguous on platforms whose system clipboard holds multiple items (macOS NSPasteboard, iOS UIPasteboard, Android ClipData). If the slot is null (e.g., a ClipboardItem produced by the customItem-no-items fallback path in read()), the call rejects with DataError to match Chromium's null-blob error categorization. - Source the web-custom-format map by looking up the corresponding system clipboard representation on the originating item, instead of the informal "Read X from the system clipboard" prose. Keeps the custom-format path consistent with the originating-item model on multi-item platforms. - Use mimeType's essence (not the full serialization with parameters) as the representations-with-resolvers map key, so getType("text/html;charset=utf-8") and getType("text/html") hit the same cached promise.
Match the concise shape of the sibling clipboard change count at read slot. Drops the redundant author-constructed clause (implied by the null initial value) and the multi-item motivation prose (already conveyed by the getType() algorithm context). Co-Authored-By: Shweta Bindal <shwetabindal@microsoft.com>
|
|
||
| 1. If |representation|'s [=representation/MIME type=] is |mimeType| and |representation|'s [=representation/isCustom=] is |isCustom|, then: | ||
|
|
||
| 1. If [=this=]'s [=ClipboardItem/clipboard change count at read=] is not null, and the current [=clipboard change count=] is not equal to [=this=]'s [=ClipboardItem/clipboard change count at read=], then [=reject=] |p| with a {{"DataError"}} {{DOMException}} in |realm|, and return |p|. |
There was a problem hiding this comment.
I'm not totally sold on DataError being the right choice to reject. Does this come form WebKit's current behavior?
Worth considering some others that might be a better fit from https://webidl.spec.whatwg.org/#idl-DOMException-error-names:
InvalidStateErrorNotAllowedErrorSecurityError
A comparison of the other things that can throw these errors and DataError might help grant some confidence of which might be the best fit.
Switch the sequence-number-mismatch and null-originating-item rejections in ClipboardItem.getType() from DataError to InvalidStateError. Across Blink, InvalidStateError is the established vocabulary for "receiver's backing resource is no longer valid" (IndexedDB transaction finished, FSA handle no longer valid, GATT disconnected, AudioContext closed). A stale clipboard snapshot is the same shape: the data isn't malformed, the binding to the system clipboard has moved on. Co-Authored-By: Shweta Bindal <shwetabindal@microsoft.com>
Spec change to defer OS clipboard reads from
clipboard.read()toClipboardItem.getType().Previously,
clipboard.read()eagerly fetched all clipboard payload bytes fromthe OS at call time. This change specifies that
clipboard.read()enumeratesonly the available format names (metadata), and the actual OS data read is
deferred to when the author calls
getType()on the resultingClipboardItem.Specific changes:
clipboard.read(): initializes each representation's data to a new unresolvedpromise instead of eagerly resolving it with OS clipboard bytes. Resolves the
existing Issue: "It should be possible to read the data asynchronously from
the system clipboard after the author calls getType".
ClipboardItem.getType(): adds a deferred OS read block that runs in parallel,reads bytes via the
os specific well-known formatalgorithm, appliessanitization (same rules as before), and queues resolution on the clipboard
task source per Infra conventions.
w3c issue line - Selective Clipboard Format Read #240
Preview | Diff