Skip to content
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

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

Open
snianu opened this issue Sep 12, 2023 · 11 comments
Open

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

snianu opened this issue Sep 12, 2023 · 11 comments

Comments

@snianu
Copy link
Contributor

snianu commented Sep 12, 2023

Reading all supported formats from the clipboard during read() takes time especially since the HTML/Image payload can be in MBs/GBs. We could defer reading of the actual data from the clipboard and populate the clipboard item object with just promises to the blob data for all supported formats. That way we can resolve the promise with the actual data from the clipboard when getType is called. But this also means that we would have to track changes to the system clipboard as any change to the formats that was read during ClipboardItem object creation, would make the object stale/invalid. It also impacts formats that were delay rendered.

In the spec[1] it explicitly states that read should read all the data from the system clipboard in step 3. When getType is called, we resolve the promise to the data that was read during Clipboarditem object creation in step 6.

Should we make a change to the spec to read the data on-demand and not upfront? This would also help in delayed rendering of formats.

Note that in Chromium, DataTransfer APIs already follow a similar pattern where the sequence number is checked to see if the clipboard data that was read during the data transfer item object creation is stale or not: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/clipboard/data_object_item.cc;drc=4c2fec5f6699ffeefd93137d2bf8c03504c6664c;l=215

@inexorabletash @evanstade @annevk @whsieh @sanketj

@snianu snianu added the Agenda+ label Sep 12, 2023
@inexorabletash
Copy link
Member

Seems reasonable to me, but I'd like to hear from other implementers, especially re: their approach to stale data.

@snianu
Copy link
Contributor Author

snianu commented Oct 12, 2023

Bot posted the resolution in an unrelated issue. Posting the relevant discussion here: w3c/edit-context#26 (comment)

The Web Editing Working Group just discussed Read Blob data for the supported formats on-demand during getType., and agreed to the following:

RESOLVED: Change API to behave as specified by Anupam above
The full IRC log of that discussion
<dandclark> topic: Read Blob data for the supported formats on-demand during getType.
<dandclark> github: https://github.com/https://github.com/w3c/clipboard-apis/issues/191
<dandclark> snianu: When we read (in Chromium) async clipboard, we read all supported formats. When authors specify getType, we return the blob
<dandclark> snianu: Has perf issues, web authors maybe only want plaintext or HTML.
<dandclark> ...: Don't need to read all formats
<dandclark> ...: Proposal is to make decision in getType, not in read
<dandclark> ...: When authors call read() we return formats supported in clipboard. But don't read actual data.
<dandclark> ...: Only read the data when authors call getType()
<dandclark> ...: In dataTransfer, already have this perf optimization. In Windows, check the sequence number, do something similar in MacOS. When clipboard changes, seq number incremented
<dandclark> ...: Can query the number to see if clipboard has changed since you read the formats
<whsieh> q+
<dandclark> ...: If doesn't match, returns empty string. Proposal here is to reject the promise.
<dandclark> whsieh: Proposal is so you read the data lazily. So this is what we do right now in webkit, with the same policy. If change count changes we'll reject the promise.
<edgar> q+
<dandclark> snianu: This matches dataTransfer, so this will make everything interoperable
<dandclark> edgar: What if the web author calls getType multiple times. Between the calls, the data was changed. What happens?
<dandclark> snianu: For dataTransfer it will be empty string every time. For this API, we reject promise.
<dandclark> snianu: In this scenario if clipboard changes between getType calls, e.g. because you switched focus and copied something, then yes we reject promise.
<dandclark> snianu: Proposal here is to reject promise for getType if clipboard has been invalidated. Use sequenceNumber on Windows, the corresponding approach on MacOS. Reject if doesn't match, resolve the promise with the data if matches.
<dandclark> RESOLVED: Change API to behave as specified by Anupam above

@evanstade
Copy link

A couple thoughts

  1. It doesn't seem like the API was designing with this concern in mind (in fact, the spec is explicit about it not working this way). If the API were designed to handle this, I would have expected the list of preferred formats to have been passed as a parameter in the read function. We could still introduce a version of read that works that way (returning only the first format in the list which exists on the clipboard). This avoids the problem of mismatched sequence numbers altogether.
  2. In lieu of that, being able to specify lazy loading as an option similar to Add unsanitized option to async clipboard API. #197 allows site to control this behavior. (It seems entirely reasonable for a site to exist that stores the returned clipboardItems and calls getType() at a later time, perhaps for example to provide "clipboard history" functionality. This site would now be broken.) It's not clear to me what the default value should be. The least likely to cause problems for existing sites is for the default to match the current behavior (not lazy loading); the most likely to be beneficial for performance overall is to make lazy loading the default.

@snianu
Copy link
Contributor Author

snianu commented Nov 21, 2023

  1. Just making sure that my understanding of this proposal is correct -- You're proposing a new read() method that takes a sequence of MIME types as parameter, and the ClipboardItem that is returned, only has the data for these MIME types. That way we don't have to check for sequence number as the data for these formats would be read upfront. Correct?
    Some concerns with this approach:
  • Does the web author have to call read({formats}) multiple times if the first call for text/html failed because the format wasn't present in the clipboard and now the author wants to read a different format? If not, then how does a web author know what formats are present on the clipboard that they can specify in the read option? If the web author has to always read all supported formats, then it kills the benefits of delay rendering and having a Promise wouldn't make much sense in getType as the data for all the formats are already present after read.

  • If text/html & text/plain formats are delay rendered by a native app during copy, but during paste, the site prefers to use text/html (if present) and add text/plain as a fallback option, then they have to specify both in the read({formats}) method. That kills the benefit of delay rendering as the native app has to produce data for both the formats during paste.

  • An example in Excel Online and Google Sheets where paste support pasting multiple options:
    Excel Online:
    image
    Google Sheet:
    image

If the app doesn't want to read the data at the time of paste but want to show paste options for all supported formats present in the clipboard, then it would be waste of resources if they implement it using the existing read() and the proposed read({formats}) methods that don't support lazy loading of data for the formats.

  1. For the clipboard history example, it is highly unlikely that the app would cache all the formats as part of the clipboard history as they are expensive to store in memory (both local and server side). On Windows only plain text is supported by-default in the clipboard history because it is not memory efficient to save images, HTML, high fidelity custom formats etc for all copy operations.
    Clipboard history app can still be implemented with the proposed lazy loading of data in getType -- it's just that the app would have to call getType for all the formats they want to save in the history.
    Note that Safari has shipped this behavior, so the site would be broken there if they aren't already calling getType multiple times for all formats they want to cache in history. With the proposed change in read() and getType, it will also help with browser interop.

@evanstade
Copy link

You're proposing a new read() method that takes a sequence of MIME types as parameter, and the ClipboardItem that is returned, only has the data for these MIME types.

No, it returns data in only one or zero formats. The one format is the first one in the list that was present on the clipboard. If the clipboard does not contain any of the provided formats, no data is returned.

Neither the Excel nor Sheets example is very motivating because the paste options are presented before the paste event has actually happened, which means that they would need the clipboard permission if they work the way you're presenting (i.e. by figuring out what types are actually available at the time of right click). Sites like Excel and Sheets should not need the clipboard permission to function. I just tested out Google Sheets and discovered that "paste special" has 5+ different options even when I only have plain text on the clipboard.

image

Seems not that disastrous to just present a broad array of options in "paste as" even without knowing what formats are actually available.

Excel can optimize itself within this framework by utilizing a web custom format which holds a single representation of the copied data, and then after paste, processing that canonical data into "function" form, "link" form, "text" form or whatever else those icons imply.

For the clipboard history example, it is highly unlikely that the app would cache all the formats as part of the clipboard history

I don't agree, but it's a toy example, and the point is that some apps could rely on existing behavior.

Note that Safari has shipped this behavior

It's unfortunate that they diverged from the spec, but that doesn't change that this could break existing sites which only care about or test on non-Safari browsers.

@snianu
Copy link
Contributor Author

snianu commented Dec 13, 2023

Neither the Excel nor Sheets example is very motivating because the paste options are presented before the paste event has actually happened

I don't think this is true. The paste options in the image below show up after the paste event is fired and the text is inserted into the DOM.
Clipboard permissions are requested before the paste option menu is shown, not after.
image

@snianu
Copy link
Contributor Author

snianu commented Jan 4, 2024

@evanstade Can we discuss your concerns in the next EditingWG call(01/11)? Wanted to make sure they are addressed before we mark this issue as resolved. (Removing the resolved for now).

@snianu snianu added Agenda+ and removed RESOLVED labels Jan 4, 2024
@evanstade
Copy link

Yes, happy to discuss there.

@snianu
Copy link
Contributor Author

snianu commented Jan 18, 2024

@evanstade We discussed with our Excel partners about the concerns you raised in the last EditingWG meeting and have some questions that we would like to discuss in the next meeting:

  1. Reading the data for a format on-demand on getType is required for delay rendering. Without this, delay rendering of formats doesn't provide any value as the callbacks for all formats would be triggered when the read() is invoked. However, just like delay rendering, we could make the lazy read of formats opt-in by adding a new option in the read() method like you mentioned in this comment. Would that be a reasonable compromise?
  2. If we make it optional, then does this lazy read get enabled by-default when a site wants to support delay rendering? e.g. if site A provides callback for couple of formats during copy and site B calls read() without providing the lazy load option, then does the lazy load get activated automatically for site B? If so, then does it make sense to provide an option when in some cases it can't be optional?

I agree that providing more delay between read() and getType() would affect the reliability of the APIs as clipboard can change between the two read steps. But, with the read-all formats approach, getType that returns a Promise to a Blob seems unnecessary as the data is already available after the read() call. We could also add a clipboardchange event so the web authors know when the clipboard has changed, but I'm not sure if it would solve the reliability issue. Thoughts?

@snianu
Copy link
Contributor Author

snianu commented Feb 7, 2024

@evanstade Prepared a document describing the interaction of on-demand read with delayed clipboard rendering feature. Please let us know what you think. Thanks!

@css-meeting-bot
Copy link
Member

The Web Editing Working Group just discussed https://github.com/w3c/clipboard-apis/issues/191.

The full IRC log of that discussion <dandclark> topic: https://github.com//issues/191
<dandclark> github: https://github.com//issues/191
<dandclark> snianu: We're still investigating and discussing what the right approach is
<dandclark> johanneswilm: Let's discuss another time
<whsieh> https://github.com//issues/137

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

No branches or pull requests

6 participants