Last Comment Bug 752402 - Blob constructor should take ArrayBufferView as a member of blobParts parameter in addition to ArrayBuffer
: Blob constructor should take ArrayBufferView as a member of blobParts paramet...
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Masatoshi Kimura [:emk]
:
: Andrew Overholt [:overholt]
Mentors:
http://dev.w3.org/2006/webapi/FileAPI...
Depends on:
Blocks: 721569 744907
  Show dependency treegraph
 
Reported: 2012-05-06 21:43 PDT by Masatoshi Kimura [:emk]
Modified: 2012-08-23 13:10 PDT (History)
6 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (11.26 KB, patch)
2012-05-07 11:11 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Splinter Review
Support ArrayBufferView as a member of blobParts parameter (9.12 KB, patch)
2012-05-13 08:45 PDT, Masatoshi Kimura [:emk]
jonas: review+
Details | Diff | Splinter Review
Support ArrayBufferView as a member of blobParts parameter (9.12 KB, patch)
2012-05-17 13:09 PDT, Masatoshi Kimura [:emk]
VYV03354: review+
Details | Diff | Splinter Review

Description Masatoshi Kimura [:emk] 2012-05-06 21:43:51 PDT
Per latest File API spec.
Comment 1 Masatoshi Kimura [:emk] 2012-05-06 21:45:24 PDT
Do we need to disable Blob constructor on aurora and beta? This is an incompatible change.
Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-05-06 22:06:34 PDT
I don't think we should disable it no. It's unclear to me if we'll actually be able to remove ArrayBuffer as a valid type here. Based on conversations with microsoft people, apparently IE10 is going ship with the Blob constructor taking ArrayBuffers (it's too late in their cycle to change). So we might be stuck with ArrayBuffer as an overload.

We'll discuss this more on the list, but I see no reason to start disabling things in our implementation at this point.
Comment 3 Masatoshi Kimura [:emk] 2012-05-07 11:11:36 PDT
Created attachment 621672 [details] [diff] [review]
patch

This patch doesn't remove ArrayBuffer support.
Comment 4 Alex Keybl [:akeybl] 2012-05-07 16:05:16 PDT
(In reply to Jonas Sicking (:sicking) from comment #2)
> I don't think we should disable it no. It's unclear to me if we'll actually
> be able to remove ArrayBuffer as a valid type here. Based on conversations
> with microsoft people, apparently IE10 is going ship with the Blob
> constructor taking ArrayBuffers (it's too late in their cycle to change). So
> we might be stuck with ArrayBuffer as an overload.
> 
> We'll discuss this more on the list, but I see no reason to start disabling
> things in our implementation at this point.

If we're not considering disabling the Blob constructor, are there any remaining next actions for FF13.
Comment 5 Masatoshi Kimura [:emk] 2012-05-07 16:07:14 PDT
Probably none. Withdrawn tracking request.
Comment 6 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-05-11 12:44:02 PDT
I don't actually think we should remove support for ArrayBuffer yet. Apparently microsoft is too late in their shipping cycle to remove support for ArrayBuffer in IE10. After which it's probably too late to remove ArrayBuffer support.

So the spec might just end up having to require support for both ArrayBuffer *and* ArrayBufferView.

I think for now we should just leave ArrayBuffer in there and see where the spec head.

Does that sound ok?
Comment 7 Masatoshi Kimura [:emk] 2012-05-11 19:19:19 PDT
Comment on attachment 621672 [details] [diff] [review]
patch

Fair enough. I'll update the patch later.
Comment 8 Masatoshi Kimura [:emk] 2012-05-13 08:45:14 PDT
Created attachment 623505 [details] [diff] [review]
Support ArrayBufferView as a member of blobParts parameter

Use of ArrayBuffer is no longer deprecated.
Comment 9 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-05-13 22:48:39 PDT
Comment on attachment 623505 [details] [diff] [review]
Support ArrayBufferView as a member of blobParts parameter

Review of attachment 623505 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great!

::: content/base/src/nsDOMBlobBuilder.cpp
@@ +263,1 @@
>        } else if (element.isString()) {

I'm not sure that this else-branch is worth it any more. JS_ValueToString should be quite fast when the passed in value is a string. I'd just remove it.
Comment 10 Masatoshi Kimura [:emk] 2012-05-17 13:09:39 PDT
Created attachment 624857 [details] [diff] [review]
Support ArrayBufferView as a member of blobParts parameter

patch for checkin
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-05-17 16:37:20 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/79d51b043d45
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-05-18 13:20:40 PDT
https://hg.mozilla.org/mozilla-central/rev/79d51b043d45
Comment 13 Arun Ranganathan 2012-08-23 13:10:03 PDT
(In reply to Jonas Sicking (:sicking) from comment #6)
> I don't actually think we should remove support for ArrayBuffer yet.
> Apparently microsoft is too late in their shipping cycle to remove support
> for ArrayBuffer in IE10. After which it's probably too late to remove
> ArrayBuffer support.
> 
> So the spec might just end up having to require support for both ArrayBuffer
> *and* ArrayBufferView.
> 
> I think for now we should just leave ArrayBuffer in there and see where the
> spec head.
> 
> Does that sound ok?

Presently, the spec. only supports ArrayBufferView.  I'm not sure whether it should also support ArrayBuffer, but if two leading implementations support ArrayBuffer I suppose that decision is already made.

Note You need to log in before you can comment on or make changes to this bug.