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

mkruisselbrink commented on this pull request.



> +<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:
+  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=].

Huh, interesting that XHR and FileReader are different in this regard. The FileReader spec has fairly clearly always done this (i.e. https://www.w3.org/TR/2009/WD-FileAPI-20091117/#readAsBinaryString says "Queue a task to dispatch a progress event called loadstart.", while XHR has always done it synchronously.

I suppose it would make sense to consider making XHR and FileReader behave the same, but that should be a separate change. As of now Firefox is clearly the odd one out not following any version of the FileReader spec that I can find. I.e. not sure what you're argument is for Firefox being correct here?

> +     [=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, 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 steps were last invoked,
+       [=queue a task=] to [=fire a progress event=] called {{progress}} at the [=context object=].
+    1. Set |chunk| to the result of [=read a chunk|reading a chunk=] from |stream| with |reader|.
+
+  1. Otherwise, if |chunk| is fulfilled with an object whose `done` property is true,
+     [=queue a task=] to run the following steps and abort this algorithm:
+    1. Set the [=context object=]'s {{FileReader/readyState}} to {{DONE}}.
+    1. Let |result| be the result of
+       [=package data=] given |bytes|, |type|, |blob|'s {{Blob/type}} and |encodingName|.

Done

> +1. If {{FileReader/readyState}} = {{FileReader/LOADING}}
+   set {{FileReader/readyState}} to {{FileReader/DONE}}
+   and {{FileReader/result}} to `null`.
+1. If there are any [=tasks=] from the [=context object=]
+   on the [=file reading task source=] in an affiliated [=queue a task|task queue=],
+   then remove those [=tasks=] from that task queue.
+1. [=terminate an algorithm|Terminate the algorithm=] for the [=read method=] being processed.
+1. [=Fire a progress event=] called {{abort}} at the [=context object=].
+1. If [=context object=]'s {{FileReader/readyState}} is not {{LOADING}},
+   [=fire a progress event=] called {{loadend}} at the [=context object=].
+
+## Packaging data ## {#packaging-data}
+
+<div algorithm="package data">
+A {{Blob}} has an associated <dfn for=Blob>package data</dfn> algorithm,
+given |bytes|, a |type|, a optional |mimeType| and a optional |encodingName|,

Done

> +
+## Packaging data ## {#packaging-data}
+
+<div algorithm="package data">
+A {{Blob}} has an associated <dfn for=Blob>package data</dfn> algorithm,
+given |bytes|, a |type|, a optional |mimeType| and a optional |encodingName|,
+switches on |type|, and runs the associated 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]].

Removed the requirement around how data URLs without media types should be treated. Also added a link to #104 to track further work in better specifying how data URLs are generated.

> +switches on |type|, and runs the associated 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.

Actually just replaced all the null's with failures in this block. That way I can avoid all the "if failure set to null" steps, and the whole thing seems like it is a lot more straight forward.

> +:: 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:
+     1. Let |type| be the result of [=parse a MIME type=] given |mimeType|.
+     1. If |type| is failure, abort these substeps.

I just merged the following step into this step, since there really is only one next step anyway.

>  
-1. Initiate a <a>read operation</a> using the <code>blob</code> argument,
-  and with the <a>synchronous flag</a> *set*.
-  If the read operation returns failure,
-  throw the appropriate exception as defined in [[#dfn-error-codes]].
-  <a>Terminate this algorithm</a>.
-1. If no error has occurred,
-  return the |result| of the <a>read operation</a> represented as a string
-  in a format determined through the <a>encoding determination</a> algorithm.
+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|.
+   If that threw an exception, abort these steps and rethrow that exception.

Thanks, removed.

> -1. Initiate a <a>read operation</a> using the <code>blob</code> argument,
-  and with the <a>synchronous flag</a> *set*.
-  If the read operation returns failure,
-  throw the appropriate exception as defined in [[#dfn-error-codes]].
-  <a>Terminate this algorithm</a>.
-1. If no error has occurred,
-  return the |result| of the <a>read operation</a> represented as a string
-  in a format determined through the <a>encoding determination</a> algorithm.
+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|.
+   If that threw an exception, abort these steps and rethrow that exception.
+1. Let |promise| be the result of [=read all bytes|reading all bytes=] from |stream| with |reader|.
+1. Wait for |promise| to be fulfilled or rejected.
+1. If |promise| fulfilled with a [=byte sequence=] |bytes|:
+  1. Return the result of [=package data=] given |bytes|, *Text*, |blob|'s {{Blob/type}} and |encoding|.
+1. Otherwise:

Done

>  
-1. Initiate a <a>read operation</a> using the <code>blob</code> argument,
-  and with the <a>synchronous flag</a> *set*.
-  If the <a>read operation</a> returns failure,
-  throw the appropriate exception as defined in [[#dfn-error-codes]].
-  <a>Terminate this algorithm</a>.
-1. If no error has occurred, return the |result| of the <a>read operation</a>
-  as a Data URL [[!RFC2397]] subject to the considerations below:
-
-  * Use the <code>blob</code>'s {{Blob/type}} attribute as part of the Data URL if it is available
-    in keeping with the Data URL specification [[!RFC2397]].
-  * If the {{Blob/type}} attribute is not available on the <code>blob</code> 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]].
+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|.
+   If that threw an exception, abort these steps and rethrow that exception.

Done

>  
-1. Initiate a <a>read operation</a> using the <code>blob</code> argument,
-  and with the <a>synchronous flag</a> *set*.
-  If the <a>read operation</a> returns failure,
-  throw the appropriate exception as defined in [[#dfn-error-codes]].
-  <a>Terminate this algorithm</a>.
-1. If no error has occurred, return the |result| of the <a>read operation</a> as an {{ArrayBuffer}}.
+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|.
+   If that threw an exception, abort these steps and rethrow that exception.

Done

>  
-1. Initiate a <a>read operation</a> using the <code>blob</code> argument,
-  and with the <a>synchronous flag</a> *set*.
-  If the <a>read operation</a> returns failure,
-  throw the appropriate exception as defined in [[#dfn-error-codes]].
-  <a>Terminate this algorithm</a>.
-1. If no error has occurred, return the |result| of the <a>read operation</a> as an {{ArrayBuffer}}.
+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|.
+   If that threw an exception, abort these steps and rethrow that exception.
+1. Let |promise| be the result of [=read all bytes|reading all bytes=] from |stream| with |reader|.
+1. Wait for |promise| to be fulfilled or rejected.
+1. If |promise| fulfilled with a [=byte sequence=] |bytes|:
+  1. Return the result of [=package data=] given |bytes|, *ArrayBuffer* and |blob|'s {{Blob/type}}.

Done

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