Closed
Bug 1259588
Opened 8 years ago
Closed 8 years ago
new File("") throws Gecko internal NS_ERROR_FAILURE
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: smaug, Assigned: shawnjohnjr)
Details
(Whiteboard: n[tw-dom] btpp-active)
Attachments
(1 file, 2 obsolete files)
If it should throw per spec, it should throw some correct exception.
Whiteboard: [tw-dom]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → shuang
Updated•8 years ago
|
Whiteboard: [tw-dom] → [tw-dom] btpp-active
Assignee | ||
Comment 1•8 years ago
|
||
It looks like it's calling File constructor for chrome process only[1][2]. I don't see File API specification defines exception that I shall throw in this case. And Chromium throws TypeError exception. [1]https://dxr.mozilla.org/mozilla-central/source/dom/webidl/File.webidl?case=true&from=File.webidl#15 [2]https://dxr.mozilla.org/mozilla-central/source/dom/base/File.cpp?case=true&from=File.cpp#625 Is it correct to throw TypeError exception if the File API specification doesn't define it?
Flags: needinfo?(bugs)
Assignee | ||
Comment 2•8 years ago
|
||
https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#ChromeConstructor Maybe we shall use '[ChromeConstructor]' in File.webidl? Or there is any historical reason we need to expose these constructor?
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Shawn Huang [:shawnjohnjr] from comment #2) > https://developer.mozilla.org/en-US/docs/Mozilla/ > WebIDL_bindings#ChromeConstructor > Maybe we shall use '[ChromeConstructor]' in File.webidl? Or there is any > historical reason we need to expose these constructor? Sorry, I missed that [Constructor] and [ChromeConstructor] are mutually exclusive.
Comment 4•8 years ago
|
||
Shawn, what you want to do is replacing: aRv.Throw(NS_ERROR_FAILURE); with: return ThrowErrorMessage(cx, MSG_ILLEGAL_CONSTRUCTOR);
Flags: needinfo?(bugs)
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #4) > Shawn, what you want to do is replacing: > > aRv.Throw(NS_ERROR_FAILURE); > > with: > > return ThrowErrorMessage(cx, MSG_ILLEGAL_CONSTRUCTOR); Thanks a lot for the hint. :) I tried the suggestion but I saw "Tab crashed" after running the test case. Looking into the crash. [Child 25866] WARNING: g_path_get_basename: assertion 'file_name != NULL' failed: 'glib warning', file /home/shawn/code/mozilla-inbound/toolkit/xre/nsSigHandlers.cpp, line 142 (process:25866): GLib-CRITICAL **: g_path_get_basename: assertion 'file_name != NULL' failed JavaScript error: file:///home/shawn/code/mozilla-inbound/obj-x86_64-pc-linux-gnu/dist/bin/components/Weave.js, line 13: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIXPCComponents_Utils.import] 0 INFO SimpleTest START 1 INFO TEST-START | dom/base/test/test_bug1259588.html ************ illegal constructor ***[Parent 25800] WARNING: pipe error (38): Connection reset by peer: file /home/shawn/code/mozilla-inbound/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 313 [Parent 25800] WARNING: pipe error (55): Connection reset by peer: file /home/shawn/code/mozilla-inbound/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 313 ###!!! [Parent][MessageChannel] Error: (msgtype=0x2C0074,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv
Assignee | ||
Comment 6•8 years ago
|
||
#0 0x00007fffea33ebb8 in mozilla::dom::GetOrCreateDOMReflectorHelper<RefPtr<mozilla::dom::File>, true>::GetOrCreate (cx=0x7fffbfb4c400, value=..., givenProto=givenProto@entry=..., rval=...) at /home/shawn/code/mozilla-inbound/obj-x86_64-pc-linux-gnu/dist/include/mozilla/dom/BindingUtils.h:1684 #1 0x00007fffea33031a in mozilla::dom::GetOrCreateDOMReflector<RefPtr<mozilla::dom::File> > (givenProto=..., rval=..., value=..., cx=<optimized out>) at /home/shawn/code/mozilla-inbound/obj-x86_64-pc-linux-gnu/dist/include/mozilla/dom/BindingUtils.h:1707 #2 mozilla::dom::FileBinding::_constructor (cx=0x7fffbfb4c400, argc=<optimized out>, vp=<optimized out>) at /home/shawn/code/mozilla-inbound/obj-x86_64-pc-linux-gnu/dom/bindings/FileBinding.cpp:1020 #3 0x00007fffeb8cd554 in js::CallJSNative (args=..., native=0x7fffea32f365 <mozilla::dom::FileBinding::_constructor(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffbfb4c400) at /home/shawn/code/mozilla-inbound/js/src/jscntxtinlines.h:235 #4 js::CallJSNativeConstructor (args=..., native=0x7fffea32f365 <mozilla::dom::FileBinding::_constructor(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffbfb4c400) at /home/shawn/code/mozilla-inbound/js/src/jscntxtinlines.h:268 #5 InternalConstruct (cx=0x7fffbfb4c400, args=...) at /home/shawn/code/mozilla-inbound/js/src/vm/Interpreter.cpp:581 #6 0x00007fffeb8bfa5c in Interpret (cx=0x7fffbfb4c400, state=...) at /home/shawn/code/mozilla-inbound/js/src/vm/Interpreter.cpp:2823 #7 0x00007fffeb8cbc8d in js::RunScript (cx=cx@entry=0x7fffbfb4c400, state=...) at /home/shawn/code/mozilla-inbound/js/src/vm/Interpreter.cpp:426
Reporter | ||
Comment 7•8 years ago
|
||
Why MSG_ILLEGAL_CONSTRUCTOR? It should be TypeError for non-content caller, because the first parameter can't be converted to a sequence
Whiteboard: [tw-dom] btpp-active → n[tw-dom] btpp-active
Comment 8•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #7) > Why MSG_ILLEGAL_CONSTRUCTOR? It should be TypeError for non-content caller, > because the first parameter can't be converted to a sequence Oh! Right!
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8742213 -
Flags: review?(amarchesini)
Comment 10•8 years ago
|
||
Comment on attachment 8742213 [details] [diff] [review] Bug 1259588 - new File("") throws TypeError exception Review of attachment 8742213 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/File.cpp @@ +621,5 @@ > const ChromeFilePropertyBag& aBag, > ErrorResult& aRv) > { > if (!nsContentUtils::ThreadsafeIsCallerChrome()) { > + aRv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>(); This should be aRv.ThrowTypeError<MSG_MISSING_ARGUMENTS>(NS_LITERAL_STRING("File")).
Attachment #8742213 -
Flags: review?(amarchesini) → review-
Assignee | ||
Updated•8 years ago
|
Attachment #8742213 -
Attachment is obsolete: true
Assignee | ||
Comment 11•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8742294 -
Flags: review?(amarchesini)
Comment 12•8 years ago
|
||
Comment on attachment 8742294 [details] [diff] [review] Bug 1259588 - new File("") throws TypeError exception (v2) Review of attachment 8742294 [details] [diff] [review]: ----------------------------------------------------------------- Good!
Attachment #8742294 -
Flags: review?(amarchesini) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8742294 -
Attachment is obsolete: true
Assignee | ||
Comment 13•8 years ago
|
||
Assignee | ||
Comment 14•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e765a071a48
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0a222ef3bf63
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•