Re: [w3c/clipboard-apis] Add clipboard IDL description. (#158)

@domenic commented on this pull request.

Much better now, awesome!

I started to move on a bit to the Clipboard algorithms, just doing read() for now. Some of the things I commented on there are probably generalizable to readText() as well. (And maybe write() and writeText().)

> +
+  A [=clipboard item=] can also optionally have a <dfn>presentation style</dfn> that helps distinguish whether apps "pasting" a [=clipboard item=] should insert the contents of an appropriate [=representation=] inline at the point of paste or if it should be treated as an attachment.
+
+  Apps that support pasting only a single [=clipboard item=] should use the first [=clipboard item=].
+  Apps that support pasting more than one [=clipboard item=] could, for example, provide a user interface that previews the contents of each [=clipboard item=] and allow the user to choose which one to paste.
+  Further, apps are expected to enumerate the [=mime type=]s of the [=clipboard item=] they are pasting and select the one best-suited for the app according to some app-specific algorithm.
+  Alternatively, an app can present the user with options on how to paste a [=clipboard item=], e.g. "paste as image" or "paste formatted text", etc.
+
+  A {{ClipboardItem}} object has an associated [=ClipboardItem/clipboard item=], which is a [=clipboard item=].
+
+  To create a {{ClipboardItem}} object, given a [=clipboard item=] |clipboardItem|'s [=relevant realm=] |realm|, run these steps:
+   1. Let |clipboardItemObject| be a new {{ClipboardItem}} with |realm|.
+
+   1. Set |clipboardItemObject|'s [=clipboard item=] to |clipboardItem|.
+
+  <a constructor lt="ClipboardItem()">constructor</a> steps for <code>new ClipboardItem(<var>items</var>, <var>options</var>)</code> are:

I believe the correct markup here is something like

```suggestion
  The <dfn lt="ClipboardItem()"><code>new ClipboardItem(<var>items</var>, <var>options</var>)</code></dfn> constructor steps are:
```

although I don't know very much about ReSpec so I might be off.

> +    1. Set |representation|'s [=mime type=] to |key|.
+
+    1. Set |representation|'s [=data=] to |value|.
+
+    1. Append |representation| to [=this=]'s [=ClipboardItem/clipboard item=]'s [=list of representations=].
+
+  <h4 attribute for=ClipboardItem lt=presentationStyle>presentationStyle</h4>
+  <p>
+   The {{ClipboardItem/presentationStyle}} getter steps are to return [=this=]'s [=ClipboardItem/clipboard item=]'s [=presentation style=].
+  </p>
+
+  <h4 attribute for=ClipboardItem lt=types>types</h4>
+  <p>
+   {{ClipboardItem/types}} getter steps are:
+
+   1. Let |types| be the result of running [=create a frozen array=].

Unfortunately this version will create a new frozen array every time, since the getter steps are run every time you do `clipboardItem.types`. I.e., this will end up with `clipboardItem.types !== clipboardItem.types`.

Instead you should move this frozen array creation into the constructor, and store it on a new internal slot, e.g. something like `A {{ClipboardItem}} object has an associated <dfn for="ClipboardItem">types array</dfn>, which is a `FrozenArray<DOMString>`. Then this can just be `Return [=this=]'s [=ClipboardItem/types array=]`.

> +  <h4 attribute for=ClipboardItem lt=presentationStyle>presentationStyle</h4>
+  <p>
+   The {{ClipboardItem/presentationStyle}} getter steps are to return [=this=]'s [=ClipboardItem/clipboard item=]'s [=presentation style=].
+  </p>
+
+  <h4 attribute for=ClipboardItem lt=types>types</h4>
+  <p>
+   {{ClipboardItem/types}} getter steps are:
+
+   1. Let |types| be the result of running [=create a frozen array=].
+   
+   1. Let |itemTypeList| be [=this=]'s [=ClipboardItem/clipboard item=]'s [=list of representations=].
+   
+   1. For each |itemType| in |itemTypeList|:
+    
+    1. Add |itemType|'s [=mime type=] to |types|.

The [=mime type=] here is a [MIME type](https://mimesniff.spec.whatwg.org/#mime-type), not a string. You want a string. To convert it, use [serialize a MIME type](https://mimesniff.spec.whatwg.org/#serialize-a-mime-type).

> +    1. Add |itemType|'s [=mime type=] to |types|.
+
+   1. Return |types|.
+  </p>
+
+  <h4 method for=ClipboardItem lt=getType(type)>getType(type)</a> must run the below steps:</h4>
+
+   1. Let |realm| be [=this=]'s [=relevant realm=].
+
+   1. Let |mimeType| be the result of [=parse a mime type=] given |type|.
+
+   1. If |mimeType| is failure, then throw a {{TypeError}}.
+
+   1. Let |itemTypeList| be [=this=]'s [=ClipboardItem/clipboard item=]'s [=list of representations=].
+
+   1. If |mimeType| is not present in |itemTypeList|'s [=representation=]'s [=mime type=], then [=a promise rejected with=] {{"NotFoundError"}} DOMException in |realm|.

These steps need to be a bit more algorithmic. I suggest an explicit loop: something like

1. For each |representation| in |itemTypeList|:
   1. If |representation|'s [=mime type=] is |mimeType|, then:
      1. ... do the stuff to return the promise, using |representation|'s [=data=].
1. Return a promise rejected with a "NotFoundError" DOMException.

Note the difference between my suggestion using a variable |representation|, introduced by the for loop, versus what you have here, which uses the *spec type* [=representation=]. You can't meaningfully get a specific value out of a type; you need an instance of the type.

>    <div class="algorithm" data-algorithm="clipboard-read">
   <h4 method for="Clipboard">read()</h4>
   The {{Clipboard/read()}} method must run these steps:
 
-   1. Let |p| be a new [=Promise=].
+   1. Let |realm| be [=this=]'s [=relevant realm=].
+
+   1. Let |p| be [=a new promise=] in |realm|.
 
    1. Run the following steps [=in parallel=]:
 
     1. Let |r| be the result of running [=check clipboard read permission=] [=in parallel=]

```suggestion
    1. Let |r| be the result of running [=check clipboard read permission=]
```

(we're already "in parallel" so this is not necessary).

>  
-    1. Let |data| be a copy of the [=system clipboard data=] represented as
-     a sequence of {{ClipboardItem}}s.
+    1. If |r| is not "granted", then [=a promise rejected with=] {{"NotAllowedError"}} DOMException in |realm|.

You need to reject |p|, like the old text did; "a promise rejected with" is something you can return, but you can't return things from in parallel.

You also need to queue a task to get back to the main thread before you can mess with main-thread objects like promises. See https://html.spec.whatwg.org/#event-loop-for-spec-authors

>  
-    1. Let |data| be a copy of the [=system clipboard data=] represented as
-     a sequence of {{ClipboardItem}}s.
+    1. If |r| is not "granted", then [=a promise rejected with=] {{"NotAllowedError"}} DOMException in |realm|.
+
+    1. Let |data| be a copy of the [=system clipboard data=] represented as [=ClipboardItems=]. For the MIME types defined in the [=mandatory data types=] list, |data| contains the sanitized copy of text/html format, but image/png format has unsanitized payload to preserve meta data.

I think you want to represent them as `[=clipboard items=]`, not `[=ClipboardItems=]`.

Then, you need to queue a task to get back to the main thread and create new `{{ClipboardItem}}`s from the `[=clipboard item=]`s. Something like:

1. Let |data| be a copy of the [=system clipboard data=] represented as [=clipboard items=]. For ...
2. [=Queue a global task=] on the ??? task source given |realm|'s [=realm/global object=] to perform the following steps:
   1. Let |items| be a `sequence<ClipboardItem>`, created in |realm|.
   2. For each [=clipboard item=] |underlyingItem| of |data|:
       1. Let |item| be a [=new=] {{ClipbardItem}}.
       2. Set |item|'s [=ClipboardItem/clipboard item=] to |underlyingItem|.
       3. Append |item| to |items|.
   3. [=Resolve=] |p| with |items|.

> +  A [=clipboard item=] is conceptually data that the user has expressed a desire to make shareable by invoking a "cut" or "copy" command.
+  For example, if a user copies a range of cells from a spreadsheet of a native application, it will result in one [=clipboard item=]. If a user copies a set of files from their desktop, that list of files will be represented by multiple [=clipboard item=]s.
+  Some platforms may support having more than one [=clipboard item=] at a time on the [=Clipboard=], while other platforms replace the previous [=clipboard item=] with the new one. 
+
+  A [=clipboard item=] has a <dfn>list of representations</dfn>, each <dfn>representation</dfn> with an associated <dfn>mime type</dfn> and <dfn>data</dfn>. In the example where the user copies a range of cells from a spreadsheet, it may be represented as an image (image/png), an HTML table (text/html), or plain text (text/plain).
+  Each of these [=mime type=]s describe a different [=representation=] of the same [=clipboard item=] at different levels of fidelity and make the [=clipboard item=] more consumable by target applications during paste.
+  Making the range of cells available as an image will allow the user to paste the cells into a photo editing app, while the text/plain format can be used by text editor apps.
+
+  A [=clipboard item=] can also optionally have a <dfn>presentation style</dfn> that helps distinguish whether apps "pasting" a [=clipboard item=] should insert the contents of an appropriate [=representation=] inline at the point of paste or if it should be treated as an attachment.
+
+  Apps that support pasting only a single [=clipboard item=] should use the first [=clipboard item=].
+  Apps that support pasting more than one [=clipboard item=] could, for example, provide a user interface that previews the contents of each [=clipboard item=] and allow the user to choose which one to paste.
+  Further, apps are expected to enumerate the [=mime type=]s of the [=clipboard item=] they are pasting and select the one best-suited for the app according to some app-specific algorithm.
+  Alternatively, an app can present the user with options on how to paste a [=clipboard item=], e.g. "paste as image" or "paste formatted text", etc.
+
+  A {{ClipboardItem}} object has an associated [=ClipboardItem/clipboard item=], which is a [=clipboard item=].

```suggestion
  A {{ClipboardItem}} object has an associated <dfn for="ClipboardItem">clipboard item</dfn>, which is a [=clipboard item=].
```

> I still haven't grasped clearly how this part of a spec should ideally look like. Shouldn't it be:

I think we're on the right track. I would say:

- IDL block
- (Optional, but recommended:) A web-developer-facing note explaining the APIs. (You already have this, although I think you moved it down to the bottom now which is not what I would recommend.)
- Definitions of concepts associated with the ClipboardItem interface, e.g. its clipboard item (this line that we're commenting on) or useful algorithms
- Method steps for each operation, and getter/setter steps for each attribute

I don't think there's a need for notes on enums and typedefs, as web developers don't need to know about them (they're not first-class concepts) and for implementers they're pretty self-explanatory.

> +  Some platforms may support having more than one [=clipboard item=] at a time on the [=Clipboard=], while other platforms replace the previous [=clipboard item=] with the new one. 
+
+  A [=clipboard item=] has a <dfn>list of representations</dfn>, each <dfn>representation</dfn> with an associated <dfn>mime type</dfn> and <dfn>data</dfn>. In the example where the user copies a range of cells from a spreadsheet, it may be represented as an image (image/png), an HTML table (text/html), or plain text (text/plain).
+  Each of these [=mime type=]s describe a different [=representation=] of the same [=clipboard item=] at different levels of fidelity and make the [=clipboard item=] more consumable by target applications during paste.
+  Making the range of cells available as an image will allow the user to paste the cells into a photo editing app, while the text/plain format can be used by text editor apps.
+
+  A [=clipboard item=] can also optionally have a <dfn>presentation style</dfn> that helps distinguish whether apps "pasting" a [=clipboard item=] should insert the contents of an appropriate [=representation=] inline at the point of paste or if it should be treated as an attachment.
+
+  Apps that support pasting only a single [=clipboard item=] should use the first [=clipboard item=].
+  Apps that support pasting more than one [=clipboard item=] could, for example, provide a user interface that previews the contents of each [=clipboard item=] and allow the user to choose which one to paste.
+  Further, apps are expected to enumerate the [=mime type=]s of the [=clipboard item=] they are pasting and select the one best-suited for the app according to some app-specific algorithm.
+  Alternatively, an app can present the user with options on how to paste a [=clipboard item=], e.g. "paste as image" or "paste formatted text", etc.
+
+  A {{ClipboardItem}} object has an associated [=ClipboardItem/clipboard item=], which is a [=clipboard item=].
+
+  To create a {{ClipboardItem}} object, given a [=clipboard item=] |clipboardItem|'s [=relevant realm=] |realm|, run these steps:

Any object creation requires a realm; e.g. creating an object in the realm of `window` is different from creating it in the realm of `frames[0]`.

However I'm unsure what this algorithm is used for. It doesn't have a `<dfn>`, so nothing else in the spec can reference it. Are you planning to use it later?

> +  Apps that support pasting only a single [=clipboard item=] should use the first [=clipboard item=].
+  Apps that support pasting more than one [=clipboard item=] could, for example, provide a user interface that previews the contents of each [=clipboard item=] and allow the user to choose which one to paste.
+  Further, apps are expected to enumerate the [=mime type=]s of the [=clipboard item=] they are pasting and select the one best-suited for the app according to some app-specific algorithm.
+  Alternatively, an app can present the user with options on how to paste a [=clipboard item=], e.g. "paste as image" or "paste formatted text", etc.
+
+  A {{ClipboardItem}} object has an associated [=ClipboardItem/clipboard item=], which is a [=clipboard item=].
+
+  To create a {{ClipboardItem}} object, given a [=clipboard item=] |clipboardItem|'s [=relevant realm=] |realm|, run these steps:
+   1. Let |clipboardItemObject| be a new {{ClipboardItem}} with |realm|.
+
+   1. Set |clipboardItemObject|'s [=clipboard item=] to |clipboardItem|.
+
+  <a constructor lt="ClipboardItem()">constructor</a> steps for <code>new ClipboardItem(<var>items</var>, <var>options</var>)</code> are:
+   1. Set [=this=]'s [=ClipboardItem/clipboard item=] to a new [=clipboard item=].
+
+   1. If |options| is empty, then set [=this=]'s [=ClipboardItem/clipboard item=]'s [=presentation style=] to "unspecified", else, set [=this=]'s [=ClipboardItem/clipboard item=]'s [=presentation style=] to |options|["presentationStyle"].

The formatting is correct, although at least in Bikeshed you can do `|options|["{{ClipboardItemOptions/presentationStyle}}"]` to get some extra nice cross-linking. It's worth trying to see if that works in ReSpec too.

Other issue: Options will never be empty, because it defaults to `{}`, and the `presentationStyle` member defaults to `"unspecified"`, in the IDL. In other words, this is all taken care of in the IDL block. So you can just simplify this step to:

> Set this's clipboard item's presentation style to options["presentationStyle"].

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/clipboard-apis/pull/158#pullrequestreview-804208801