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

Delayed clipboard rendering API shape. #423

Open
snianu opened this issue Apr 11, 2023 · 15 comments
Open

Delayed clipboard rendering API shape. #423

snianu opened this issue Apr 11, 2023 · 15 comments
Labels
Delayed-Clipboard-Rendering Ability to delay the generation of a particular payload until it is needed by the target application RESOLVED

Comments

@snianu
Copy link
Contributor

snianu commented Apr 11, 2023

From #417 There are few comments about the shape of the new delayed rendering API in the async clipboard API. Creating this issue to continue the discussion here.
Explainer: https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/DelayedClipboard/DelayedClipboardRenderingExplainer.md
Proposed solution: Add a callback to ClipboardItemData
Discussions related to this:
@annevk asked:

Yes it does run immediately, but you don't have to call resolve() right away. You'd only call resolve(blob) once you're actually ready, which can be later on in the event loop based on some event. Am I misunderstanding something or were promises not understood?

I guess what you're saying is that there's currently no event for such a request?

This led to the following discussions about the shape of the API:

To be pedantic, this could be done with promises with the addition of another signal, e.g. an event as @annevk mentions. I think this would be a poor developer experience compared to the callback approach proposed above. Here's what it could look like, just to demonstrate:

navigator.clipboard.write(new ClipboardItem({
  'text/html': new Promise(resolve => {
    // somehow this event target is scoped to this clipboard item?
    // event will only be fired once, even if paste happens again?
    some_target.addEventListener('some_event_type', async e => {
      // are we the type handler that's actually desired here?
      if (e.requestedType === 'text/html') {
        // do a bunch of stuff here, probably async
        resolve(results);
      }
    };
  }),
  /* repeat the above for every supported type, but we'll only ever call resolve() for one Promise */
}));

There may be a cleaner way, but IMHO a callback seems much cleaner.

A callback definitely seems like the way.

Is there any value in having the clipboard item data ever be a Promise? In other words, rather than this:

typedef (Blob or DOMString) ClipboardItemValue;
callback ClipboardItemValueCallback = ClipboardItemValue ();
typedef Promise<(ClipboardItemValue or ClipboardItemValueCallback)> ClipboardItemData;
constructor(record<DOMString, ClipboardItemData> items);

I might expect this:

typedef (Blob or DOMString) ClipboardItemValue;
callback ClipboardItemValueCallback = (ClipboardItemValue or Promise<ClipboardItemValue>) ();
typedef (ClipboardItemValue or ClipboardItemValueCallback) ClipboardItemData;
constructor(record<DOMString, ClipboardItemData> items);

That signature lets the value be provided immediately, deferred, or deferred and resolved asynchronously.
Is there any value in having the clipboard item data ever be a Promise?

Yes, because we want to allow sites to construct the data asynchronously, but eagerly. (Which is how the API currently behaves.) I believe what we actually would like is:

typedef (Blob or DOMString) ClipboardItemValue;
callback ClipboardItemValueCallback = Promise<ClipboardItemValue>();
typedef (ClipboardItemValueCallback or Promise<ClipboardItemValue>) ClipboardItemData;
constructor(record<DOMString, ClipboardItemData> items);

However, it's apparently not possible for a Promise to be part of a union. Thus I think the next best option is to be able to provide additional data to the ClipboardItem with setter methods rather than in the ctor.

Is there any value in having the clipboard item data ever be a Promise?

Yes, because we want to allow sites to construct the data asynchronously, but eagerly.

Right, that makes sense. Four options, then:
Eager: a DOMString or Blob Eager, but asynchronous: a Promise of a DOMString or Blob Deferred: a callback returning a DOMString or Blob Deferred, but asynchronous: a callback returning a Promise of a DOMString or Blob
However, it's apparently not possible for a Promise to be part of a union.

If that's the case, then the callback will have to return a promise. That seems okay, but I think it'd be more ergonomic for the application developer to return a value synchronously if they want to.
Are these suggested alternative ctor param types? If so, they can't be or'd because of the promise. But I agree with the premise they could all be useful.

I agree it would be nicer not to have to wrap your argument with Promise.resolve() but it also doesn't seem too awful, and it's what the API was launched with, presumably because of the Promise union problem.

Not alternative params, no, just trying to enumerate the usage scenarios.

@evanstade @sanketj @tilgovi @inexorabletash @anaskim @whsieh

@snianu snianu added the Delayed-Clipboard-Rendering Ability to delay the generation of a particular payload until it is needed by the target application label Apr 11, 2023
@snianu
Copy link
Contributor Author

snianu commented Dec 12, 2023

It looks like we still need a resolution for this issue.

There are mainly three different API shapes for delay rendering support:

typedef (DOMString or Blob) ClipboardItemValue;
callback ClipboardItemValueCallback = ClipboardItemValue();
typedef Promise<(ClipboardItemValue or ClipboardItemValueCallback)> ClipboardItemData;

This provides a callback for the formats being delay rendered. It's ergonomical to the web authors and doesn't have any race issue when formats are written to the clipboard as the browsers can check which formats have callback associated with them and which ones have data already populated during write() before the formats are written to the system clipboard.

navigator.clipboard.write(new ClipboardItem({
'text/html': new Promise(resolve => {
// somehow this event target is scoped to this clipboard item?
// event will only be fired once, even if paste happens again?
some_target.addEventListener('some_event_type', async e => {
// are we the type handler that's actually desired here?
if (e.requestedType === 'text/html') {
// do a bunch of stuff here, probably async
resolve(results);
}
};
}),
/* repeat the above for every supported type, but we'll only ever call resolve() for one Promise */
}));

This has several issues:

  • It is not ergonomical.
  • Not sure how the sites can select which formats can be delay rendered during write(). Without this info we have to assume all the formats are delay rendered?

Add a new method to ClipboardItem called addDelayedWriteCallback, that takes in a map of formats to callbacks(addDelayedWriteCallback's).

Issues with this approach:

  • It adds overhead to the web author (it is a new function for them to learn) and can create confusion when having the same format(s) repeated in both the ClipboardItem constructor and in addDelayedWriteCallback's map

We would like to propose the first option:

typedef (DOMString or Blob) ClipboardItemValue;
callback ClipboardItemValueCallback = ClipboardItemValue();
typedef Promise<(ClipboardItemValue or ClipboardItemValueCallback)> ClipboardItemData;

@snianu snianu added the Agenda+ Agenda item to be inserted in the Editing TF meeting queue label Dec 12, 2023
@snianu
Copy link
Contributor Author

snianu commented Dec 15, 2023

RESOLUTION: Option 1 with the callback approach is the resolution for delay rendering
Minutes
6:56 PM snianu: documented in the latest comment various API shapes
6:59 PM smaug: could we let one to use streams
7:01 PM with stream we could avoid quite a memory overhead
7:01 PM snianu: can we get resolution on having callback to let authors specify what formats can be delay rendered?
7:02 PM RESOLUTION: to have callback (Option 1)

@snianu snianu added RESOLVED and removed Agenda+ Agenda item to be inserted in the Editing TF meeting queue labels Dec 15, 2023
@saschanaz
Copy link
Member

saschanaz commented Dec 15, 2023

Hmm, I was not fully understanding this issue back in the meeting, as my focus was on a different issue. To reiterate what @smaug---- said with some code, can this be:

navigator.clipboard.write(new ClipboardItem({
  'text/html': new ReadableStream({
    pull(controller) {
      // This will only be called when the consumer reads it, so this is still about "have callback".
      controller.enqueue(bytes);
    },
    type: "bytes",
  }, { highWaterMark: 0 }),
}));

I think this can coexist with callback, though.

@tilgovi
Copy link

tilgovi commented Dec 27, 2023

To coexist with callbacks, if I understand the union issue correctly, that would have to be:

navigator.clipboard.write(new ClipboardItem({
  'text/html': Promise.resolve(new ReadableStream({
    pull(controller) {
      // This will only be called when the consumer reads it, so this is still about "have callback".
      controller.enqueue(bytes);
    }),
    type: "bytes",
  }, { highWaterMark: 0 })),
}));

No?

@saschanaz
Copy link
Member

I'm not sure why promise would be needed there?

@tilgovi
Copy link

tilgovi commented Dec 27, 2023

My understanding is that we can't have a union of Promise and anything. Otherwise, we might have implemented the callback as just a function rather than a promise of a function. No?

@saschanaz
Copy link
Member

saschanaz commented Dec 27, 2023

Having Promise in Web IDL doesn't mean that the user needs to explicitly pass Promise, the IDL layer will automatically wrap it with a promise.

@tilgovi
Copy link

tilgovi commented Dec 27, 2023

Oh, that's great!

@snianu
Copy link
Contributor Author

snianu commented Jan 2, 2024

Hmm, I was not fully understanding this issue back in the meeting, as my focus was on a different issue. To reiterate what @smaug---- said with some code, can this be:

navigator.clipboard.write(new ClipboardItem({
  'text/html': new ReadableStream({
    pull(controller) {
      // This will only be called when the consumer reads it, so this is still about "have callback".
      controller.enqueue(bytes);
    },
    type: "bytes",
  }, { highWaterMark: 0 }),
}));

I think this can coexist with callback, though.

What would the IDL look like? Can Blob APIs be used to stream data instead? Basically, in the callback, can the web authors use the streaming APIs to write data into the Blob? If so, then I'm not sure if this option would be helpful to add to the ClipboardItem IDL -- creates complexity in how the different chunks of data get written to the clipboard when the format is being requested during paste.

@saschanaz
Copy link
Member

Can Blob APIs be used to stream data instead?

One can convert blobs into streams and streams into blobs, but a blob itself is by design a readonly snapshot object that does not change after its construction. Not sure that's the proper answer for your question, could you provide some example in your mind if not?

Basically, in the callback, can the web authors use the streaming APIs to write data into the Blob?

One cannot "write data into the (existing) blob" in any way as Blob is designed to be constant. But you can create a new blob from a stream: new Response(stream).blob()

@snianu
Copy link
Contributor Author

snianu commented Jan 2, 2024

One can convert blobs into streams and streams into blobs

Right, that is why I'm confused if a ReadableStream would add any value to this API when it already allows Blob data to be returned from the callback.

But you can create a new blob from a stream: new Response(stream).blob()

Sorry for the confusion, but that is what I meant in can the web authors use the streaming APIs to write data into the Blob? comment.

@smaug----
Copy link

Using a stream could let implementations to optimize memory usage, as an example.

@saschanaz
Copy link
Member

saschanaz commented Jan 2, 2024

Right, that is why I'm confused if a ReadableStream would add any value to this API when it already allows Blob data to be returned from the callback.

Having to get a memory blob (which is what you get when converting from stream) means potentially having to consume huge memory. Having stream can reduce that. And also the consumer can get the first chunk of bytes earlier without waiting for the complete data.

I think another important question is, though: does any OS support reading from clipboard progressively?

@smaug----
Copy link

Even if OSes didn't support that, streaming from the content/renderer process to the process which would then interact with the OS could save quite a bit memory in many cases.

@snianu
Copy link
Contributor Author

snianu commented Jan 2, 2024

Tagging few people who would be interested in this discussion.
@evanstade @inexorabletash @sanketj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Delayed-Clipboard-Rendering Ability to delay the generation of a particular payload until it is needed by the target application RESOLVED
Projects
None yet
Development

No branches or pull requests

4 participants