Closed
Bug 231324
Opened 21 years ago
Closed 21 years ago
<method><body/></method> breaks implementation, but not content, handlers
Categories
(Core :: XBL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: WeirdAl, Assigned: hyatt)
Details
Attachments
(2 files)
|
1.73 KB,
application/vnd.mozilla.xul+xml
|
Details | |
|
1.32 KB,
patch
|
bryner
:
review+
bryner
:
superreview+
|
Details | Diff | Splinter Review |
A body element in the XBL binding with zero child nodes will break the adding of
properties, fields, and methods to a bound node. Testcase coming up in a few
moments.
Reproducible: Always
Steps to reproduce: Click on the button in the attached testcase.
Note that including any child nodes, even a space (<body> </body>), will not
cause this bug. I would expect that <body/> would be treated identically to
<body> </body>.
Also note that the anonymous content and handlers still apply.
|
Reporter | |
Comment 1•21 years ago
|
||
|
||
Comment 2•21 years ago
|
||
This happens because nsXBLProtoImplMethod::CompileMember throws if there is no
mUncompiledMethod. We have a few options here:
1) Just return NS_OK. This won't define the method on the object, but won't
keep other methods from being defined.
2) Synthesize an empty function body in this case. (Behavior more like
<body> </body>, possibly).
3) Try to take down the whole binding somehow when this happens.
Thoughts?
|
||
Comment 3•21 years ago
|
||
I'd prefer to go for case 1) - don't we already do that for empty bodies?
if (body.IsEmpty())
return NS_OK;
|
|
||
Comment 4•21 years ago
|
||
Note that a real error (eg OOM) will _still_ leave the binding "sorta
attached"....
|
|
||
Updated•21 years ago
|
Attachment #139372 -
Flags: superreview?(bryner)
Attachment #139372 -
Flags: review?(bryner)
|
Assignee | |
Comment 5•21 years ago
|
||
There's nothing wrong with having an empty body, and IMO we need not define the function on the
class prototype, but should ensure the binding attaches ok.
|
|
||
Comment 6•21 years ago
|
||
That's what the patch does.
|
||
Comment 7•21 years ago
|
||
Comment on attachment 139372 [details] [diff] [review]
So like this
r+sr=bryner
Attachment #139372 -
Flags: superreview?(bryner)
Attachment #139372 -
Flags: superreview+
Attachment #139372 -
Flags: review?(bryner)
Attachment #139372 -
Flags: review+
You need to log in
before you can comment on or make changes to this bug.
Description
•