Re: [w3c/FileAPI] Rewrite FileReader definitions. (#118)

annevk commented on this pull request.

Thanks, I'm a little worried about all the promise usage in parallel, but I guess since we don't have a low-level stream model that's hard to get away from. Fetch also has this problem... Maybe @domenic has thoughts.

> @@ -251,6 +253,25 @@ Their [=deserialization step=], given |serialized| and |value|, are:
 
 2. Set |value|'s underlying byte sequence to |serialized|.\[[ByteSequence]].
 
+<div algorithm="get stream">
+A {{Blob}} has an associated <dfn for=Blob>get stream</dfn> algorithm,
+given a |blob|, and runs the following steps:

It's not associated with a `Blob` if it takes a `Blob`. So instead you want "The get stream algorithm, given a `Blob` _blob_, must run these steps:" or some such.

>  };
 </pre>
 
+<div algorithm="read operation">
+A {{FileReader}} has an associated <dfn id=readOperation>read operation</dfn> algorithm,
+given |blob|, a |type| and an optional |encodingName|,
+runs the following steps:
+
+1. Set the [=context object=]'s {{FileReader/result}} to `null`.
+1. Set the [=context object=]'s {{FileReader/error!!attribute}} to `null`.
+1. Let |stream| be the result of calling [=get stream=] on |blob|.
+1. Let |reader| be the result of [=get a reader|getting a reader=] from |stream|.
+1. Let |bytes| by an empty [=byte sequence=].
+1. Let |chunk| be the result of [=read a chunk|reading a chunk=] from |stream| with |reader|.
+1. Let |isFirstChunk| be true.
+1. [=In parallel=], while true:

This isn't really in parallel since you operate on promises from the main thread. Maybe it's okay for now though, but doesn't seem like the correct setup long term.

cc @domenic 

> +
+1. Set the [=context object=]'s {{FileReader/result}} to `null`.
+1. Set the [=context object=]'s {{FileReader/error!!attribute}} to `null`.
+1. Let |stream| be the result of calling [=get stream=] on |blob|.
+1. Let |reader| be the result of [=get a reader|getting a reader=] from |stream|.
+1. Let |bytes| by an empty [=byte sequence=].
+1. Let |chunk| be the result of [=read a chunk|reading a chunk=] from |stream| with |reader|.
+1. Let |isFirstChunk| be true.
+1. [=In parallel=], while true:
+  1. Wait for |chunk| to be fulfilled or rejected.
+  1. If |chunk| is fulfilled, and |isFirstChunk| is true,
+     [=queue a task=] to [=fire a progress event=] called {{loadstart}} at the [=context object=].
+  1. Set |isFirstChunk| to false.
+
+  1. If |chunk| is fulfilled with an object whose `done` property is false and whose `value`
+     property is a `Uint8Array` object, then run these steps:

s/ run these steps//

> +1. Let |stream| be the result of calling [=get stream=] on |blob|.
+1. Let |reader| be the result of [=get a reader|getting a reader=] from |stream|.
+1. Let |bytes| by an empty [=byte sequence=].
+1. Let |chunk| be the result of [=read a chunk|reading a chunk=] from |stream| with |reader|.
+1. Let |isFirstChunk| be true.
+1. [=In parallel=], while true:
+  1. Wait for |chunk| to be fulfilled or rejected.
+  1. If |chunk| is fulfilled, and |isFirstChunk| is true,
+     [=queue a task=] to [=fire a progress event=] called {{loadstart}} at the [=context object=].
+  1. Set |isFirstChunk| to false.
+
+  1. If |chunk| is fulfilled with an object whose `done` property is false and whose `value`
+     property is a `Uint8Array` object, then run these steps:
+    1. Let |bs| be the [=byte sequence=] represented by the `Uint8Array` object.
+    1. Append |bs| to |bytes|.
+    1. If roughly 50ms have passed since these subsubsubsteps were last invoked,

steps*

> +: DataURL
+:: Return |bytes| as a DataURL [[!RFC2397]] subject to the considerations below:
+
+  * Use |mimeType| as part of the Data URL if it is available
+    in keeping with the Data URL specification [[!RFC2397]].
+  * If |mimeType| is not available return a Data URL without a media-type. [[!RFC2397]].
+    Data URLs that do not have media-types [[RFC2046]]
+    must be treated as plain text by conforming user agents. [[!RFC2397]].
+
+: Text
+:: 1. Let |encoding| be null.
+   1. If the |encodingName| is present, set |encoding| to the result of
+      [=getting an encoding=] from |encodingName|.
+   1. If the [=getting an encoding=] step above returns failure, set |encoding| to null.
+   1. If |encoding| is null, and |mimeType| is present,
+      and it uses a Charset Parameter [[RFC2046]],

This should use https://mimesniff.spec.whatwg.org/#parse-a-mime-type.

> +      and it uses a Charset Parameter [[RFC2046]],
+      set |encoding| to the result of <a>getting an encoding</a>
+      for the portion of the Charset Parameter that is a <i>label</i> of an encoding.
+
+      <div class="example">
+        If `blob` has a {{Blob/type}} attribute of `text/plain;charset=utf-8`
+        then <a>getting an encoding</a> is run using `"utf-8"` as the label.
+        Note that user agents must parse and extract the portion of the Charset Parameter
+        that constitutes a *label* of an encoding.
+      </div>
+   1. If the [=getting an encoding=] steps above return failure,
+      then set |encoding| to null.
+   1. If |encoding| is null, then set |encoding| to [=UTF-8=].
+   1. [=Decode=] |bytes| using fallback encoding |encoding|, and return the result.
+
+   The above steps are also known as the <dfn>encoding determination</dfn>.

The way this concept is referenced seems to indicate some kind of misunderstanding as to what actually takes place. I suggest we take this opportunity to remove it.

-- 
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/FileAPI/pull/118#pullrequestreview-213819580