Last Comment Bug 1164310 - Implement MS's proposal for a reduced subset of the new FileSystem API
: Implement MS's proposal for a reduced subset of the new FileSystem API
Status: RESOLVED FIXED
: dev-doc-needed, devrel-needed
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
-- normal with 4 votes (vote)
: mozilla42
Assigned To: Jonathan Watt [:jwatt]
:
: Andrew Overholt [:overholt]
Mentors:
https://lists.w3.org/Archives/Public/...
: 846931 876480 (view as bug list)
Depends on: 907707 1173320 1173390 1177688 1188818 1190036
Blocks: 1188880
  Show dependency treegraph
 
Reported: 2015-05-12 18:56 PDT by Jonathan Watt [:jwatt]
Modified: 2016-09-03 03:07 PDT (History)
36 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected
fixed


Attachments
part 1 - Make the code for bypassing mobile security checks more general so that it can be used on non-mobile (3.68 KB, patch)
2015-07-02 09:37 PDT, Jonathan Watt [:jwatt]
amarchesini: review+
Details | Diff | Splinter Review
part 2 - Implement an abstraction for a rooted filesystem for non-mobile devices (14.38 KB, patch)
2015-07-02 09:38 PDT, Jonathan Watt [:jwatt]
amarchesini: review+
Details | Diff | Splinter Review
part 3 - Allow the DirState of blobs to be set explicitly (2.82 KB, patch)
2015-07-02 09:39 PDT, Jonathan Watt [:jwatt]
amarchesini: review+
Details | Diff | Splinter Review
part 4 - Implement the new HTMLInputElement API including the new Promise returning GetFilesAndDirectories (15.39 KB, patch)
2015-07-02 09:49 PDT, Jonathan Watt [:jwatt]
amarchesini: review+
Details | Diff | Splinter Review
part 5 - Implement new anonymous content and layout pieces for directory picking via input elements (12.09 KB, patch)
2015-07-02 10:36 PDT, Jonathan Watt [:jwatt]
tnikkel: review+
Details | Diff | Splinter Review
part 6 - Implement the new Promise returning DataTransfer.getFilesAndDirectories() API (7.76 KB, patch)
2015-07-02 12:30 PDT, Jonathan Watt [:jwatt]
amarchesini: review+
Details | Diff | Splinter Review

Description User image Jonathan Watt [:jwatt] 2015-05-12 18:56:48 PDT
+++ This bug was initially created as a clone of Bug #846931 +++

Bug 846931 has proceeded on the basis of allowing users to select directories while having the browser then act in a way that is compatible with legacy websites.

To avoid a confusing morphing of that bug I'm spinning of this new bug to cover implementing Microsoft's proposal for a subset of the new FileSystem API, a proposal which would not "just work" with existing webpages but rather would require opt-in from new pages. More info on that proposal can be found here:

https://lists.w3.org/Archives/Public/public-webapps/2015AprJun/0245.html
Comment 1 User image brandon 2015-06-10 10:52:47 PDT
Thank you for spinning off this bug!

This API would be very useful to us. We currently have uploading functionality (for very large directory hierarchies) that is only supported in Chrome and Opera.

The proposed API would be able to replace the Chrome/Opera one, and it would allow us to support those uploads in Firefox and Edge as well.
Comment 2 User image Jonathan Watt [:jwatt] 2015-07-02 09:37:13 PDT
Created attachment 8628919 [details] [diff] [review]
part 1 - Make the code for bypassing mobile security checks more general so that it can be used on non-mobile
Comment 3 User image Jonathan Watt [:jwatt] 2015-07-02 09:38:21 PDT
Created attachment 8628920 [details] [diff] [review]
part 2 - Implement an abstraction for a rooted filesystem for non-mobile devices
Comment 4 User image Jonathan Watt [:jwatt] 2015-07-02 09:39:40 PDT
Created attachment 8628921 [details] [diff] [review]
part 3 - Allow the DirState of blobs to be set explicitly
Comment 5 User image Jonathan Watt [:jwatt] 2015-07-02 09:49:55 PDT
Created attachment 8628925 [details] [diff] [review]
part 4 - Implement the new HTMLInputElement API including the new Promise returning GetFilesAndDirectories
Comment 6 User image Jonathan Watt [:jwatt] 2015-07-02 10:36:03 PDT
Created attachment 8628959 [details] [diff] [review]
part 5 - Implement new anonymous content and layout pieces for directory picking via input elements
Comment 7 User image Jonathan Watt [:jwatt] 2015-07-02 12:30:38 PDT
Created attachment 8629016 [details] [diff] [review]
part 6 - Implement the new  Promise returning DataTransfer.getFilesAndDirectories() API
Comment 8 User image Andrea Marchesini [:baku] 2015-07-08 04:03:33 PDT
Comment on attachment 8628920 [details] [diff] [review]
part 2 - Implement an abstraction for a rooted filesystem for non-mobile devices

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

lgtm!

::: dom/filesystem/OSFileSystem.cpp
@@ +36,5 @@
> +
> +void
> +OSFileSystem::Init(nsPIDOMWindow* aWindow)
> +{
> +  MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!");

MOZ_ASSERT(!mWindow, "No duple Init() calls"); Could it make sense?

@@ +45,5 @@
> +nsPIDOMWindow*
> +OSFileSystem::GetWindow() const
> +{
> +  MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!");
> +  if (!mWindow) {

this is not needed.
I think you can just do:

return mWindow;

@@ +63,5 @@
> +{
> +  // The concept of "safe files" is specific to the Device Storage API where
> +  // files are only "safe" if they're of a type that is appropriate for the
> +  // area of device storage that is being used.
> +  MOZ_ASSERT(false, "Don't use OSFileSystem with the Device Storage API");

MOZ_CRASH

@@ +73,5 @@
> +{
> +  // The concept of "safe directory" is specific to the Device Storage API
> +  // where a directory is only "safe" if it belongs to the area of device
> +  // storage that it is being used with.
> +  MOZ_ASSERT(false, "Don't use OSFileSystem with the Device Storage API");

MOZ_CRASH

::: dom/filesystem/OSFileSystem.h
@@ +43,5 @@
> +} // namespace dom
> +} // namespace mozilla
> +
> +#endif // mozilla_dom_OSFileSystem_h
> +

extra line at the bottom of this file.
Comment 9 User image Andrea Marchesini [:baku] 2015-07-10 08:26:27 PDT
Comment on attachment 8628925 [details] [diff] [review]
part 4 - Implement the new HTMLInputElement API including the new Promise returning GetFilesAndDirectories

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

::: dom/html/HTMLInputElement.cpp
@@ +3564,5 @@
> +    if (target &&
> +        target->GetParent() == this &&
> +        target->IsRootOfNativeAnonymousSubtree() &&
> +        target->HasAttr(kNameSpaceID_None, nsGkAtoms::directory)) {
> +      MOZ_ASSERT(Preferences::GetBool("dom.input.dirpicker", false),

I really would like to see a test for this :)

@@ +4827,5 @@
> +
> +bool
> +HTMLInputElement::IsFilesAndDirectoriesSupported() const
> +{
> +  // This method is supposed to return true if a file and directory picker

80chars?

@@ +4838,5 @@
> +void
> +HTMLInputElement::ChooseDirectory(ErrorResult& aRv)
> +{
> +  if (mType != NS_FORM_INPUT_FILE) {
> +    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);

return;

@@ +4853,5 @@
> +static already_AddRefed<OSFileSystem>
> +MakeOrReuseFileSystem(const nsAString& aNewLocalRootPath,
> +                      OSFileSystem* aFS,
> +                      nsPIDOMWindow* aWindow)
> +{

MOZ_ASSERT(aWindow);

@@ +4891,5 @@
> +    p->MaybeReject(NS_ERROR_OUT_OF_MEMORY);
> +    return p.forget();
> +  }
> +
> +  nsPIDOMWindow* window = OwnerDoc()->GetInnerWindow(); // XXXjwatt correct thing?

why this comment?
I guess yes, you want to use the inner window.
Comment 10 User image Andrea Marchesini [:baku] 2015-07-10 08:30:15 PDT
Comment on attachment 8629016 [details] [diff] [review]
part 6 - Implement the new  Promise returning DataTransfer.getFilesAndDirectories() API

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

::: dom/events/DataTransfer.cpp
@@ +8,5 @@
>  #include "mozilla/BasicEvents.h"
>  
>  #include "DataTransfer.h"
>  
> +#include "mozilla/dom/Directory.h"

why here on top and not with the other mozilla/dom/ headers?

@@ +833,5 @@
> +static already_AddRefed<OSFileSystem>
> +MakeOrReuseFileSystem(const nsAString& aNewLocalRootPath,
> +                      OSFileSystem* aFS,
> +                      nsPIDOMWindow* aWindow)
> +{

MOZ_ASSERT(aWindow);

@@ +904,5 @@
> +      nsDependentSubstring basename = Substring(path, leafSeparatorIndex);
> +      fs = MakeOrReuseFileSystem(dirname, fs, window);
> +      filesAndDirsSeq[i].SetAsDirectory() = new Directory(fs, basename);
> +    } else {
> +      filesAndDirsSeq[i].SetAsFile() = mFiles->Item(i);

This is equal to what I just reviewed in the previous patch. Correct?
Wondering if we can unify these 2 blocks (this and HTMLInputElement::GetFilesAndDirectories.
Comment 11 User image Andrew Overholt [:overholt] 2015-07-16 12:08:22 PDT
*** Bug 846931 has been marked as a duplicate of this bug. ***
Comment 12 User image silverwind 2015-07-21 05:36:02 PDT
*** Bug 876480 has been marked as a duplicate of this bug. ***
Comment 16 User image Jet Villegas (:jet) 2015-07-21 16:19:33 PDT
\o/
Comment 17 User image Carsten Book [:Tomcat] 2015-07-22 05:05:34 PDT
https://hg.mozilla.org/mozilla-central/rev/a82eb6004f08
Comment 18 User image Florian Bender 2015-07-26 03:18:49 PDT
Is this pref-controlled / en-/disabled by default?
Comment 19 User image Jonathan Watt [:jwatt] 2015-07-26 07:26:38 PDT
It's controlled by the pref dom.input.dirpicker which is on in Nightly only, for now.
Comment 20 User image silverwind 2015-07-26 11:00:05 PDT
Is the drag an drop part of this finished? I seem to get a File when dropping a directory. The <input> part is working great, though!
Comment 21 User image silverwind 2015-07-27 14:15:23 PDT
To further clarify: When dropping a single directory event.dataTransfer.getFilesAndDirectories() seems to return a File instead of an expected Directory. I can file a bug if you like.
Comment 22 User image Jonathan Watt [:jwatt] 2015-07-29 03:23:47 PDT
Seems to work for me. Can you file a bug please?
Comment 23 User image Oliver Frietsch 2015-11-18 07:31:32 PST
Sorry for maybe asking a newbie question about the release process, but how can I find out when this will be in Firefox _stable_? I've tested Nightly 45 and really liked this feature, but Firefox 42 does not ship it despite this ticket has a flag "status-firefox42: fixed".
Comment 24 User image Boris Zbarsky [:bz] (still a bit busy) 2015-11-18 07:54:45 PST
That's a good question, not a newbie one.  ;)

This bug implemented the functionality behind a preference ("dom.input.dirpicker" to be precise).  This preference is currently only set to true by default in nightly builds.

Bug 1188880 (blocked by this bug) tracks enabling in release builds.  Note that it has several unresolved dependencies blocking it.
Comment 25 User image jakebrumby@gmail.com 2016-04-27 19:27:37 PDT
It's currently not possible to upload a folder into Firefox to upload it to cloud storage (Google Drive, One Drive, Sirv, Dropbox or others). When will this be possible? The need for folder uploading is growing and has already received many votes on this related thread:

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

It should be possible to upload folders by both drag and drop and file browsing.

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