Closed
Bug 914381
Opened 12 years ago
Closed 12 years ago
File pickers are broken
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 26
People
(Reporter: wesj, Assigned: wesj)
Details
Attachments
(2 files)
|
3.59 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
|
2.70 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
File pickers crash on nightly. Fallout from bug 894840. Our file picker is implemented in JS, and calling File(nsIFile) in JS components creates a nsDOMMultipartFile internally:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMBlobBuilder.h#23
nsDOMMultipartFile hasn't implemented mozFullPathInternal and so falls back to the default one, which returns "" and causes badness in HTMLInputElement.cpp.
|
Assignee | |
Comment 1•12 years ago
|
||
This fixes multipartfile to support mozGetFullPathInternal if the file was initialized with an nsIFile. Working on a test...
Assignee: nobody → wjohnston
Attachment #801855 -
Flags: review?(khuey)
Comment on attachment 801855 [details] [diff] [review]
Patch v1
Review of attachment 801855 [details] [diff] [review]:
-----------------------------------------------------------------
::: content/base/src/nsDOMBlobBuilder.cpp
@@ +257,5 @@
>
> +NS_IMETHODIMP
> +nsDOMMultipartFile::GetMozFullPathInternal(nsAString &aFilename)
> +{
> + if (!mIsFromNsiFile || mBlobs.Length() == 0) {
|| mBlobs.IsEmpty()
@@ +265,5 @@
> + nsIDOMBlob* blob = mBlobs.ElementAt(0).get();
> + nsDOMFileFile* file = static_cast<nsDOMFileFile*>(blob);
> + if (!file) {
> + return nsDOMFile::GetMozFullPathInternal(aFilename);
> + }
Get rid of this block
@@ +267,5 @@
> + if (!file) {
> + return nsDOMFile::GetMozFullPathInternal(aFilename);
> + }
> +
> + return file->GetMozFullPathInternal(aFilename);
and just do
return mBlobs[0]->GetMozFullPathInternal(aFilename);
Attachment #801855 -
Flags: review?(khuey) → review+
|
|
Assignee | |
Comment 3•12 years ago
|
||
Test. Also, mBlob[0]->GetMozFullPathInternal won't work since this is a TArray of nsCOMPtr<nsIDOMBlob>. I'm just going with:
nsIDOMBlob* blob = mBlobs[0];
nsDOMFileFile* file = static_cast<nsDOMFileFile*>(blob);
return file->GetMozFullPathInternal(aFilename);
unless you have something you'd rather.
Attachment #802427 -
Flags: review?(khuey)
(In reply to Wesley Johnston (:wesj) from comment #3)
> Created attachment 802427 [details] [diff] [review]
> Test
>
> Test. Also, mBlob[0]->GetMozFullPathInternal won't work since this is a
> TArray of nsCOMPtr<nsIDOMBlob>. I'm just going with:
>
> nsIDOMBlob* blob = mBlobs[0];
> nsDOMFileFile* file = static_cast<nsDOMFileFile*>(blob);
> return file->GetMozFullPathInternal(aFilename);
>
> unless you have something you'd rather.
Ah, yes. You should QI to nsIDOMFile (which should never fail) and then call it.
http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIDOMFile.idl#74
Attachment #802427 -
Attachment is patch: true
Comment on attachment 802427 [details] [diff] [review]
Test
Review of attachment 802427 [details] [diff] [review]:
-----------------------------------------------------------------
::: content/base/test/chrome/test_bug914381.html
@@ +9,5 @@
> + <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
> + <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
> +</head>
> +<body>
> +<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=650776">Mozilla Bug 650776</a>
That's wrong!
@@ +12,5 @@
> +<body>
> +<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=650776">Mozilla Bug 650776</a>
> +<p id="display"></p>
> +<div id="content" style="display: none">
> +
nit: extra whitespace
Attachment #802427 -
Flags: review?(khuey) → review+
|
|
Assignee | |
Comment 6•12 years ago
|
||
|
||
Comment 7•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•