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 #925

Closed
1 task done
snianu opened this issue Jan 3, 2024 · 10 comments
Closed
1 task done

Delayed Clipboard Rendering #925

snianu opened this issue Jan 3, 2024 · 10 comments
Assignees
Labels
Resolution: satisfied with concerns The TAG is satisfied with this work overall but requires changes Review type: CG early review An early review of general direction from a Community Group Topic: editing

Comments

@snianu
Copy link

snianu commented Jan 3, 2024

こんにちは 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: w3c/editing#423
Privacy issue for web custom formats: w3c/editing#439
General discussions: w3c/editing#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

@snianu snianu added Progress: untriaged Review type: CG early review An early review of general direction from a Community Group labels Jan 3, 2024
@torgo
Copy link
Member

torgo commented Jan 15, 2024

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
Copy link
Author

snianu commented Jan 23, 2024

@torgo Sorry for the delayed response.

considering you've highlighted a potential w3c/editing#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
Copy link
Contributor

hober commented Jan 24, 2024

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

dandclark pushed a commit to MicrosoftEdge/MSEdgeExplainers that referenced this issue Jan 26, 2024
…privacy (#742)

Add section discussing the question of how to handle the tab/browser closing when there are still unresolved delay-rendered formats.

Address TAG comments from w3ctag/design-reviews#925 (comment) by adding section expanding on privacy.
@torgo torgo modified the milestones: 2024-01-15-week, 2024-02-05-week Feb 4, 2024
@snianu
Copy link
Author

snianu commented Feb 8, 2024

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 w3c/editing#439 (comment) for more info. Thanks again for the feedback!

@plinss
Copy link
Member

plinss commented Feb 12, 2024

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
Copy link
Author

snianu commented Feb 12, 2024

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: whatwg/webidl#1278 (comment)

@plinss
Copy link
Member

plinss commented Feb 13, 2024

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
Copy link
Author

snianu commented Feb 13, 2024

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: w3c/editing#417 (comment)

@plinss
Copy link
Member

plinss commented Feb 13, 2024

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.

@plinss plinss removed this from the 2024-02-12-week milestone Feb 26, 2024
@plinss plinss added this to the 2024-02-26-week milestone Feb 26, 2024
@martinthomson
Copy link
Contributor

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.

@martinthomson martinthomson added the Resolution: satisfied with concerns The TAG is satisfied with this work overall but requires changes label Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: satisfied with concerns The TAG is satisfied with this work overall but requires changes Review type: CG early review An early review of general direction from a Community Group Topic: editing
Projects
None yet
Development

No branches or pull requests

6 participants