Closed Bug 231324 Opened 20 years ago Closed 20 years ago

<method><body/></method> breaks implementation, but not content, handlers

Categories

(Core :: XBL, defect)

Product:
Core
Component:
XBL
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: WeirdAl, Assigned: hyatt)

Details

Attachments

(2 files)

testcase
1.73 KB, application/vnd.mozilla.xul+xml
Details
So like this
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.
Attached file testcase — 

    
    

    
  
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?

    
    

    
  
I'd prefer to go for case 1) - don't we already do that for empty bodies?
if (body.IsEmpty())
  return NS_OK;

    
    

    
  

      
      
      
      

        Attached patch
          
          
        So like this
        — 
          — Splinter Review
      

    


  
Note that a real error (eg OOM) will _still_ leave the binding "sorta
attached"....

        Attachment #139372 -
        Flags: superreview?(bryner)

        Attachment #139372 -
        Flags: review?(bryner)

    
    

    
  
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.

    
    

    
  
That's what the patch does.

    
    

    
  
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+

    
    

    
  
Fixed.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.