w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
328 stars 55 forks source link

Delayed Clipboard Rendering #925

Closed snianu closed 7 months ago

snianu commented 9 months ago

こんにちは TAG-さん!

I'm requesting a TAG review of Delayed Clipboard Rendering.

Delayed clipboard rendering is the ability to delay the generation of a particular payload until it is needed by the target applications. It is useful when source applications support several clipboard formats, some or all of which are time-consuming to render.

Further details:

You should also know that...

Existing discussions on the API shape: https://github.com/w3c/editing/issues/423 Privacy issue for web custom formats: https://github.com/w3c/editing/issues/439 General discussions: https://github.com/w3c/editing/issues/417

We'd prefer the TAG provide feedback as (please delete all but the desired option):

🐛 open issues in our GitHub repo for each point of feedback

torgo commented 8 months ago

Hi @snianu considering you've highlighted a potential issue around privacy - it feels like that should be drawn out in the explainer? It's good to see some positive signals from another implementer. We're a bit confused whether this proposal is intending to cover the cases where something has been coped from a native app and pasted into a web app, or the other way around, or whether it's intended only for copying from one web app to another web app. We think it might be only for web-to-web or web-to-native - is that correct? Can you clarify?

snianu commented 8 months ago

@torgo Sorry for the delayed response.

considering you've highlighted a potential https://github.com/w3c/editing/issues/439 - it feels like that should be drawn out in the explainer?

Sounds good. I'll add this issue to the explainer.

We think it might be only for web-to-web or web-to-native - is that correct? Can you clarify?

It is only applicable if copy (not paste) is being performed in a web app. The destination app for paste doesn't matter as the system clipboard would call back into the source app during the paste operation to read the data for the requested format.

hober commented 8 months ago

I left this comment: https://github.com/MicrosoftEdge/MSEdgeExplainers/pull/742/files#r1464976569 I then got pointed to w3c/editing#439

snianu commented 7 months ago

Discussed with the EditingWG and we came to a consensus on the privacy issue that we will be supporting delayed clipboard rendering for just the built-in formats that aren't tied to any app ecosystem. Please see https://github.com/w3c/editing/issues/439#issuecomment-1934719705 for more info. Thanks again for the feedback!

plinss commented 7 months ago

Discussed during a breakout today with @hober and @martinthomson. We're happy to see the privacy concerns addressed (though we do see this approach as somewhat kicking the can down the road if more formats are exposed later).

We do have some concerns about the API shape:

typedef (DOMString or Blob) ClipboardItemValue; // should this be `Promise<ClipboardItemValue>` instead?
callback ClipboardItemValueCallback = ClipboardItemValue(); // this should take a `DOMString type` argument so you don't need a closure always
typedef Promise<(ClipboardItemValue or ClipboardItemValueCallback)> ClipboardItemData; // A promise here is a bit strange.  I might accept that you have three things: (DOMString or Blob), Promise<(DOMString or Blob)>, or a callback.

[SecureContext, Exposed=Window] // should this be transferrable?
interface ClipboardItem {
  constructor(record<DOMString, ClipboardItemData> items,
              optional ClipboardItemOptions options = {});

  readonly attribute PresentationStyle presentationStyle;  // what populates this? options?
  readonly attribute FrozenArray<DOMString> types;

  Promise<Blob> getType(DOMString type); // maybe just `get(DOMString type)`
};
snianu commented 7 months ago

Just wanted to note that the only changes we're making in ClipboardItem interface IDL for delayed clipboard rendering is the addition of a callback: callback ClipboardItemValueCallback = ClipboardItemValue();

Many of the IDL definitions for ClipboardItem interface are existing changes: https://w3c.github.io/clipboard-apis/#clipboard-item-interface

typedef (DOMString or Blob) ClipboardItemValue; // should this be Promise<ClipboardItemValue> instead?

The reason why we don't have a Promise here is because IDL compiler doesn't support union of promises. Here is the relevant issue for this: https://github.com/whatwg/webidl/issues/1278#issuecomment-1461691793

plinss commented 7 months ago

If that's the case, then why do you even have to add a callback at all? Couldn't the author simply pass an unresolved promise to the ClipboardItem constructor and the promise gets resolved at paste time?

e.g.:

async generateExpensiveBlob(): Promise<Blob> {
    return new Blob(...);
}

const clipboardItemInput = new ClipboardItem({
                            'text/plain': generateExpensiveBlob(),
                           });
navigator.clipboard.write([clipboardItemInput]);

Gets you delayed rendering, no?

snianu commented 7 months ago

async generateExpensiveBlob(): Promise { return new Blob(...); }

The primary purpose of delayed rendering is to not produce the data at all for a format that is never used on paste. In this case, the web authors have to be ready with the data when the promise executor function is run on write, correct?

Here is some discussion related to it: https://github.com/w3c/editing/issues/417#issuecomment-1461652377

plinss commented 7 months ago

Right, never mind me, I was confusing this with something else (working on too many projects these days).

Note though that we did recommend that the callback gets the type passed to it so that the a single callback could handle multiple types without creating extra closures.

Also during our discussion, were were questioning if the callback itself should return a promise (to make writing callbacks that need to do async processing easier). At the least the result of the callback should be awaited to make a promise return possible.

martinthomson commented 7 months ago

To conclude here, as this is an early review, we're OK with the resolution of the capability-probing thing (deferral) and are content with the explanation about the use of promises (our bad for missing that). We do think that there are opportunities missed -- in particular, the callback could return a promise also -- but don't see any concrete problems.