Closed Bug 374334 Opened 15 years ago Closed 14 years ago

Uncaught exception with cloneNode method and frameset element

Categories

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

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: martijn.martijn, Assigned: WeirdAl)

Details

(Keywords: regression, testcase)

Attachments

(4 files, 1 obsolete file)

Attached file testcase
See testcase, I'm getting this uncaught exception in current trunk builds:
Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMHTMLFrameSetElement.cloneNode]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)" 

I should get an alert with "[object HTMLFrameSetElement]", which I get with current branch builds.

This regressed between 2005-10-27 and 2005-10-28:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-10-27+04&maxdate=2005-10-28+06&cvsroot=%2Fcvsroot
Not sure which which bug might have caused this: bug 311827 or bug 308270 or bug 264308.
So, because there's an error in the script for the onclick attribute, everything unwinds until it ultimately causes the cloneNode operation to fail.

The error is not reported the second time around (very interesting).
The top six lines of this stack are identical.
Flags: blocking1.9?
So someone on the callstack should not be propagating the error here. Probably nsEventListenerManager::AddScriptEventListener or maybe nsGenericHTMLElement::AfterSetAttr. Additionally it would be good if nsJSContext::CompileEventHandler threw a better error so that whoever is stopping it could do so easier.

Alex, do you want to have a go at this one?
Assignee: general → ajvincent
Flags: blocking1.9? → blocking1.9+
Yeah, I'll take a look at it sometime over the next 3-7 days.
Status: NEW → ASSIGNED
This patch does fix the bug, but I don't know if using a DOM error code is appropriate for nsJSEnvironment.cpp.  jst, what do you think we should return here?  If we need a new error code, which .h file should define it and what should we call it?
Hmm, good questions. How about just using NS_ERROR_INVALID_VALUE in this case? Not ideal, but it's an existing error code that we could just use with enough of a similar meaning that we could pull that off. I'd think at least.
jst: NS_ERROR_INVALID_VALUE doesn't exist according to lxr.  Did you mean NS_ERROR_ILLEGAL_VALUE?
Yeah, duh. I mixed up NS_ERROR_INVALID_ARG and NS_ERROR_ILLEGAL_VALUE.
Attached patch updated patchSplinter Review
per jst's comments.
Attachment #261078 - Attachment is obsolete: true
Attachment #262068 - Flags: superreview?
Attachment #262068 - Flags: review?
Attachment #262068 - Flags: superreview?(jonas)
Attachment #262068 - Flags: superreview?
Attachment #262068 - Flags: review?(jonas)
Attachment #262068 - Flags: review?
Comment on attachment 262068 [details] [diff] [review]
updated patch

Jonas, if you have any problems with this patch, please speak up now :)

r+sr=jst
Attachment #262068 - Flags: superreview?(jonas)
Attachment #262068 - Flags: superreview+
Attachment #262068 - Flags: review?(jonas)
Attachment #262068 - Flags: review+
Whiteboard: [checkin needed]
We ought to mochikit this test.
Flags: in-testsuite?
Keywords: checkin-needed
Whiteboard: [checkin needed]
Checking in dom/src/base/nsJSEnvironment.cpp;
new revision: 1.327; previous revision: 1.326
Checking in content/events/src/nsEventListenerManager.cpp;
new revision: 1.276; previous revision: 1.275
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta1
Flags: in-testsuite?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.