Note: There are a few cases of duplicates in user autocompletion which are being worked on.

File constructor should take nsIFile

RESOLVED FIXED in mozilla6

Status

()

Core
DOM
--
enhancement
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

({dev-doc-complete})

Trunk
mozilla6
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Created attachment 531960 [details] [diff] [review]
Patch
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Attachment #531960 - Flags: review?(jonas)
Keywords: dev-doc-needed
Please declare rv closer to its use, and note that you shadow it in the string case.
Comment on attachment 531960 [details] [diff] [review]
Patch

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

::: content/base/src/nsDOMFile.cpp
@@ +682,5 @@
>  
> +    // If it's not a JSObject, give up
> +    JSObject* obj = JSVAL_TO_OBJECT(aArgv[0]);
> +    if (!obj)
> +      return NS_ERROR_UNEXPECTED;

This doesn't actually work. If you hand JS_TO_OBJECT something that isn't an object it'll assert and perform an invalid cast.

But I believe the JSVAL_IS_PRIMITIVE is enough to ensure that you have an object. So just remove the if-check.

@@ +701,5 @@
> +      return NS_ERROR_XPC_BAD_CONVERT_JS;
> +    }
> +
> +    nsCOMPtr<nsILocalFile> localFile;
> +    nsresult rv = NS_NewLocalFile(xpcomStr,

Remove the 'nsresult' here, as Ms2ger pointed out.

@@ +706,5 @@
> +                                  PR_FALSE, getter_AddRefs(localFile));
> +    NS_ENSURE_SUCCESS(rv, rv);
> +
> +    file = do_QueryInterface(localFile, &rv);
> +    NS_ENSURE_SUCCESS(rv, rv);

Generally speaking, don't use nsresults together with do_QIs, simply nullcheck the result instead.

Though in this case an assert should be enough.
Attachment #531960 - Flags: review?(jonas) → review+
I changed the JSVAL_IS_PRIMITIVE guard to JSVAL_IS_OBJECT.

Other comments addressed.

http://hg.mozilla.org/mozilla-central/rev/190b0bff3507
http://hg.mozilla.org/mozilla-central/rev/0b84815ad48e
Severity: normal → enhancement
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
OS: Windows 7 → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Version: unspecified → Trunk
Updated documentation:

https://developer.mozilla.org/en/Extensions/Using_the_DOM_File_API_in_chrome_code
https://developer.mozilla.org/en/DOM/File

And linked to from Firefox 6 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.