Closed Bug 1202964 Opened 4 years ago Closed 4 years ago

Use the correct nsIFilePicker API for directory picking in HTMLInputElement::nsFilePickerShownCallback::Done

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file)

It turns out that our different widget backends implement
nsIFilePicker::GetFiles differently. On OS X the returned nsISimpleEnumerator
will enumerate an item if the picker was created using 'modeOpen' or
'modeGetFolder'. (Because the cocoa version of nsFilePicker::Show appends to
mFiles in these cases.) For the other widget backends, though, the
nsISimpleEnumerator will not enumerate anything.

This bites us in HTMLInputElement::nsFilePickerShownCallback::Done, where currently we call GetFiles for
the nsIFilePicker::modeGetFolder case. The correct thing to do here is to call
GetFile instead.
Attached patch GetFile2Splinter Review
Attachment #8658507 - Flags: review?(amarchesini)
The nsBaseFilePicker.cpp change cribs the code that I previously wrote further up that file for nsBaseFilePickerEnumerator::GetNext.
Component: Document Navigation → DOM
This code is only hit when e10s is disabled FWIW.
Comment on attachment 8658507 [details] [diff] [review]
GetFile2

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

::: widget/nsBaseFilePicker.cpp
@@ +349,5 @@
>    }
>  
> +  nsRefPtr<File> domFile = File::CreateFromFile(mParent, localFile);
> +  domFile->Impl()->SetIsDirectory(mMode == nsIFilePicker::modeGetFolder);
> +  nsCOMPtr<nsIDOMBlob>(domFile).forget(aDomfile);

I think that if you do:

nsRefPtr<Blob> domFile = File::CreateFromFile(...
...
domFile.forget(aDomFile);

should work without casting.
Attachment #8658507 - Flags: review?(amarchesini) → review+
https://hg.mozilla.org/mozilla-central/rev/b91ede451160
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.