Last Comment Bug 669437 - Implement BlobBuilder.getFile
: Implement BlobBuilder.getFile
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Jonas Sicking (:sicking)
:
Mentors:
Depends on: 669433
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-05 13:07 PDT by Jonas Sicking (:sicking)
Modified: 2012-06-30 11:07 PDT (History)
6 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to fix (9.45 KB, patch)
2011-07-05 13:07 PDT, Jonas Sicking (:sicking)
khuey: review+
Details | Diff | Review

Description Jonas Sicking (:sicking) 2011-07-05 13:07:41 PDT
Created attachment 544038 [details] [diff] [review]
Patch to fix

Bug summary is pretty self explanatory. getFile takes a filename and an optional content type.
Comment 1 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2011-07-07 10:00:35 PDT
Comment on attachment 544038 [details] [diff] [review]
Patch to fix

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

r=me with two things:

1. Change the right UUID
2. Add a test to ensure that sending a Blob through FormData and sending a Blob with an empty filename send the same data (this should be the case, right?)

::: content/base/public/nsIDOMFile.idl
@@ +62,5 @@
>                                        [optional] in long long end,
>                                        [optional] in DOMString contentType);
>  };
>  
> +[scriptable, builtinclass, uuid(43c9856d-f2a7-4720-bca7-c731da24d99a)]

Why are you changing this?

@@ +72,5 @@
>    // This performs no security checks!
>    [noscript] readonly attribute DOMString mozFullPathInternal;
>  };
>  
> +[scriptable, builtinclass, uuid(c4a77171-039b-4f84-97f9-820fb51626af)]

And why this one didn't change?
Comment 2 Jonas Sicking (:sicking) 2011-07-11 19:45:21 PDT
Checked in to inbound:

http://hg.mozilla.org/integration/mozilla-inbound/rev/6c328da30bed

Thanks for the quick review!
Comment 3 Mounir Lamouri (:mounir) 2011-07-12 03:55:44 PDT
Merged:
http://hg.mozilla.org/mozilla-central/rev/6c328da30bed
Comment 4 Davit Barbakadze 2011-08-20 05:09:35 PDT
"Add a test to ensure that sending a Blob through FormData and sending a Blob with an empty filename send the same data (this should be the case, right?)"

Not sure, but is what you say related to this one here: https://bugzilla.mozilla.org/show_bug.cgi?id=680423
Comment 5 Davit Barbakadze 2011-09-03 07:15:31 PDT
Lines 357-360 have a comment:

// NB: This is a willful violation of the spec.  The spec says that
// the existing contents of the BlobBuilder should be included
// in the next blob produced.  This seems silly and has been raised
// on the WHATWG listserv.

Interesting note, but where in the spec this is mentioned, can't find, anyone quick link?... Sorry for slight offtopic.
Comment 6 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2011-09-03 07:18:48 PDT
(In reply to Davit Barbakadze from comment #5)
> Lines 357-360 have a comment:
> 
> // NB: This is a willful violation of the spec.  The spec says that
> // the existing contents of the BlobBuilder should be included
> // in the next blob produced.  This seems silly and has been raised
> // on the WHATWG listserv.
> 
> Interesting note, but where in the spec this is mentioned, can't find,
> anyone quick link?... Sorry for slight offtopic.

Well, it's more in what the spec doesn't say.  http://dev.w3.org/2009/dap/file-system/file-writer.html#widl-BlobBuilder-getBlob-Blob-in-DOMString-contentType makes no mention of clearing the existing contents of the BlobBuilder, and IMO it should.
Comment 7 Jean-Yves Perrier [:teoli] 2011-10-18 05:19:27 PDT
In order to document this, I was looking in the spec where BlobBuilder.getFile() is defined. I didn't found anything in the latest Editor's Draft: http://dev.w3.org/2009/dap/file-system/file-writer.html

What am I missing?
Comment 8 Eric Shepherd [:sheppy] 2011-10-18 07:09:09 PDT
According to comments above, it's not part of the spec but is apparently something we think should be. So we should document it and label it with the non-standard template for now.
Comment 9 Jonas Sicking (:sicking) 2011-10-18 09:10:08 PDT
Actually, I've been convinced by various people that this function is a bad idea, so we'll likely remove it before long. So not sure it's worth spending time on documenting.
Comment 10 Eric Shepherd [:sheppy] 2011-10-18 09:11:43 PDT
Is there a bug on removing getFile()? I need to keep aware of the status of that to be sure the docs get updated (or not) as appropriate based on the final decision there.
Comment 11 Davit Barbakadze 2011-10-25 04:39:50 PDT
@Jonas Sicking, can you share some arguments for getFile being a bad idea?
Comment 12 Jonas Sicking (:sicking) 2011-10-25 09:39:29 PDT
Both google and microsoft preferred a solution where the File interface always represents blobs backed by a OS-filesystem-file. That makes the distinction between Blobs and Files more meaningful and makes it easier to add more metadata to File later.

Note that this does not mean that Blobs which aren't Files can't also be backed by a OS-filesystem-file. It just means that that metadata wouldn't be available in that case.
Comment 13 Florian Scholz [:fscholz] (MDN) 2012-06-30 11:07:59 PDT
This interface is deprecated and marked as such in the docs. I've added getFile() for completeness as it's implemented since Fx 8.0:
https://developer.mozilla.org/en/DOM/BlobBuilder

Make sure to add dev-doc-needed to any follow-up bugs which will remove it.

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