Blob support for Zip file contents

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: overholt, Assigned: baku)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

Trunk
mozilla17
x86_64
Linux
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [games:p1])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

5 years ago
Zip files stored as blobs should allow for access to files contained within them.  These files would be interacted with as blobs themselves.  Jonas also suggests being able to create a zip of a collection of blobs as an interesting use case.
Spec please.

Updated

5 years ago
Blocks: 710398

Updated

5 years ago
Whiteboard: [games:p1]
(Assignee)

Comment 2

5 years ago
This is what I wrote yesterday. Maybe it's a good starting point for a more technical discussion.

2 points:

. consider it as work-in-progress
. feel free to say: we need something completely different

*Goals*

Provide the ability to read the list of files contained in an archive file and to have access to them as Blob objects.

*Status*

https://bugzilla.mozilla.org/show_bug.cgi?id=772434

*Example*

function startRead() {

  var file = document.getElementById('file').files[0];

  // Let's assume that this new Archive API supports serveral archive formats (this code is bad...)
  var formatSupported = [ '.zip', '.tar.gz', '.tgz', '.gz', '.bz2', '.jar' /* ... */ ];
  for (var i = 0; i < formatSupported.length; ++i) {
    if (file.indexOf(formatSupported[i]) != -1) {
      openArchive(file);
      return;
    }
  }

  alert("This archive file is not supported");
}

function openArchive(file) {
  // The ArchiveReader object works with Blob objects:
  var archiveReader = new ArchiveReader(file);

  // Any request is asyncronous:
  var handler = archiveReader.getFilenames();
  handler.onsuccess = success;
  handler.onerror = errorHandler;

  // Multiple requests can run at the same time:
  var handler2 = archiveReader.getFile("images/background.png");
  handler2.onsuccess = fileSuccess;
  handler2.onerror = errorHandler;
}

// The getFilenames handler receives a DOMString[] as result:
function success() {
  // this.result is a DOMString[]:
  for (var i = 0; i < this.result.length; ++i) {
    // ...
  }
}

// The GetFile handler receives a File/Blob object (and it can be used with FileReader API):
function fileSuccess() {
  // var reader = FileReader();
  // reader.readAsText(this.result, ...);
}

function errorHandler(evt) {
  if (evt.error.name == "NotReadbleErr") {
    // ...
  } else {
    // TODO: something else
  }
}

*API*

This is a draft of the API written in WebIDL:

interface ArchiveRequest : DOMRequest
{
  void abort();

  [TreatNonCallableAsNull] attribute Function? onprogress;
}

// Asyncronous API:

[Constructor(Blob blob)]
interface ArchiveReader
{
  // async read methods
  ArchiveRequest getFilenames();
  ArchiveRequest getFile(DOMString filename);
};

// Syncronous API:

[Constructor(Blob blob)]
interface ArchiveReaderSync
{
  DOMString[] getFilenames();
  Blob getFile(DOMString filename);
};
(Assignee)

Comment 3

5 years ago
Created attachment 643597 [details] [diff] [review]
first implementation - backup

I'm just using bugzilla as a backup system for my code.
(Assignee)

Comment 4

5 years ago
Created attachment 644705 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch
Attachment #643597 - Attachment is obsolete: true
Attachment #644705 - Flags: review?(jst)
(Assignee)

Updated

5 years ago
Keywords: dev-doc-needed
(Assignee)

Comment 5

5 years ago
Created attachment 646344 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch

code indented.
Assignee: nobody → amarchesini
Attachment #644705 - Attachment is obsolete: true
Attachment #644705 - Flags: review?(jst)
Attachment #646344 - Flags: review?(jst)
Comment on attachment 646344 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch

- In nsDOMArchiveRequest::GetFilenamesResult():

+  JS_FreezeObject(aCx, array);

We should check the return value here and throw an error if this call fails (returns false).

r=jst with that fixed!
Attachment #646344 - Flags: review?(jst) → review+
Comment on attachment 646344 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch

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

You can drop the 'ns' and 'nsDOM' prefixes on everything in the file namespace.

nsDOMArchiveZipFile should inherit from nsDOMFileCC, not nsDOMFile.

There's also a problem here.  You can grab an nsDOMArchiveZipFile and postMessage it to a worker thread, and which point Gecko will abort because we'll try to AddRef a cycle collected object off the main thread.  janv's FileHandle stuff has the same problem.  So somebody needs to figure out how that's going to work ...
Attachment #646344 - Flags: review-
See comment 7.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #7)
> Comment on attachment 646344 [details] [diff] [review]
[...]
> There's also a problem here.  You can grab an nsDOMArchiveZipFile and
> postMessage it to a worker thread, and which point Gecko will abort because
> we'll try to AddRef a cycle collected object off the main thread.  janv's
> FileHandle stuff has the same problem.  So somebody needs to figure out how
> that's going to work ...

Did Jan's FileHandle stuff already land? If so I would say that this belongs in a followup bug, and it should IMO not block this patch from landing.
Yes, it already landed.  I'll take that to a followup, but I'd still like to see the inheritance change and the nsDOM prefixes dropped before landing.
Depends on: 778023
(Assignee)

Comment 11

5 years ago
Created attachment 646444 [details] [diff] [review]
Bug 772434 - Blob support for Zip file contents, r=jst

objects renamed + nsDOMFileCC inherited.
I sent the patch to try server just to be sure it compiles and passes the tests on windows, android & C too (but I don't see any reason why it should fail).
Attachment #646344 - Attachment is obsolete: true
Attachment #646444 - Flags: review?(khuey)
Attachment #646444 - Flags: review?(jst)
Comment on attachment 646444 [details] [diff] [review]
Bug 772434 - Blob support for Zip file contents, r=jst

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

If all you did is change the names and the class it inherits from, I don't think this needs another review.
Attachment #646444 - Flags: review?(khuey)
Attachment #646444 - Flags: review?(jst)
I went to land this but decided to build it locally first, and turns out this doesn't build when applied to a current trunk build. NS_OUTPARAM needs to be removed (was removed on trunk), and ArchiveRequest.cpp doesn't compile due to not knowing what CCParticipantVTable is. Likely fallout from the last inheritance changes here.
(Assignee)

Comment 14

5 years ago
Created attachment 646699 [details] [diff] [review]
Bug 772434 - Blob support for Zip file contents, r=jst
Attachment #646444 - Attachment is obsolete: true
Attachment #646699 - Flags: checkin?
Landed on inbound!

https://hg.mozilla.org/integration/mozilla-inbound/rev/5bff8785ab1b
Would be great if someone could write a hacks.mozilla.org article about this API.
https://hg.mozilla.org/mozilla-central/rev/5bff8785ab1b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Depends on: 781155
Depends on: 781425
Attachment #646699 - Flags: checkin?

Comment 18

5 years ago
I believe this is non-standard, isn't it? Does it build on any standardized spec?
Depends on: 793434
Depends on: 795930

Comment 19

5 years ago
See also https://bugzilla.mozilla.org/show_bug.cgi?id=681967, not sure if this one was a follow-up or not but there might be insightful stuff in the other thread.
Depends on: 809567
Where is this feature with regards to a spec and web standards?
(In reply to Martin Best (:mbest) from comment #20)
> Where is this feature with regards to a spec and web standards?

Nowhere.
Did we ship this to the web?
(Assignee)

Comment 23

4 years ago
Yes, but it's disabled by default. Bug 795930
You need to log in before you can comment on or make changes to this bug.