Closed Bug 222864 Opened 21 years ago Closed 21 years ago

[FIX]textarea created through javascript is messed up

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.6alpha

People

(Reporter: mozilla, Assigned: bzbarsky)

Details

Attachments

(2 files)

testcase
126 bytes, text/html
Details
Proposed patch
1.85 KB, patch
peterv
: review+
jst
: superreview+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030807
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030807

Adding a TEXTAREA using javascript produces visually two textarea's, with none
of the two accessible nor usable

<html>
 <body>
  <script>
    document.body.appendChild( document.createElement( "TEXTAREA" ) );
  </script>
 </body>
</html>

Reproducible: Always

Steps to Reproduce:
1. open browser
2. use mini example in details




Expected Results:  
show one working textarea
Attached file testcase — 

    
    

    
  
So the problem seems to be the following:

1)  The content node insertion code calls BeginUpdate(), the sink flushes existing
    updates.
2)  The node is inserted.  This triggers XBL stuff, with the following stack:

#16 0x41728eae in nsXBLPrototypeHandler::BindingAttached(nsIDOMEventReceiver*) (
    this=0x834fa20, aReceiver=0x87ef910)
    at
/home/bzbarsky/mozilla/xlib/mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp:510
#17 0x41712ad2 in nsXBLPrototypeBinding::BindingAttached(nsIDOMEventReceiver*) (
    this=0x834ed68, aReceiver=0x87ef910)
    at
/home/bzbarsky/mozilla/xlib/mozilla/content/xbl/src/nsXBLPrototypeBinding.cpp:383
#18 0x4170e994 in nsXBLBinding::ExecuteAttachedHandler() (this=0x86b90a8)
    at /home/bzbarsky/mozilla/xlib/mozilla/content/xbl/src/nsXBLBinding.cpp:845
#19 0x4173412b in nsBindingManager::ProcessAttachedQueue() (this=0x86d7ee0)
    at /home/bzbarsky/mozilla/xlib/mozilla/content/xbl/src/nsBindingManager.cpp:955
#20 0x413a5cd1 in nsCSSFrameConstructor::ContentInserted(nsIPresContext*,
nsIContent*, nsIFrame*, nsIContent*, int, nsILayoutHistoryState*, int)
(this=0x8660910, 
    aPresContext=0x87a97c8, aContainer=0x87a1910, aContainerFrame=0x0,
aChild=0x85f3400, 
    aIndexInContainer=2, aFrameState=0x0, aInContentReplaced=0)
    at
/home/bzbarsky/mozilla/xlib/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp

where the binding we call ExecuteAttachedHandler() on is the "scrollbar"
binding.  The BindingAttached code calls into the JS event handler on the
binding (<constructor>, probably).  This ends up calling back into
nsHTMLDocument::ResolveName, which calls FlushPendingNotifications.  Oops.  This
calls back into HTMLContentSink::FlushPendingNotifications, which calls
FlushTags().  But now the textarea node is in the DOM, though EndUpdate() has
not been called for its insertion yet.  So we call ContentAppended on it, create
a frame for it, then the whole thing unwinds, we finish creating the first frame
for it, etc.  We end up with two frames for the same node.

The right solution, imo, is for the content sink ignore calls to
FlushPendingNotifications while in the middle of an update.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Proposed patch
        — 
          — Splinter Review
      

    


  

    
    

    
  
Comment on attachment 133648 [details] [diff] [review]
Proposed patch

peterv, jst, can you review?  Are there any testcases I should test with this
patch that you can think of?  I'd like to add all such to the regression
tests...

        Attachment #133648 -
        Flags: superreview?(jst)

        Attachment #133648 -
        Flags: review?(peterv)

    
    

    
  
Taking.
Assignee: dom_bugs → bzbarsky
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Summary: textarea created throught javascript is messed up → [FIX]textarea created throught javascript is messed up
Target Milestone: --- → mozilla1.6alpha

    
    

    
  
Comment on attachment 133648 [details] [diff] [review]
Proposed patch

No testcases that would excersize this in a useful way come to mind off the top
of my head, so sr=jst as is.

        Attachment #133648 -
        Flags: superreview?(jst) → superreview+

    
    

    
  
Fixing typo in summary.
Summary: [FIX]textarea created throught javascript is messed up → [FIX]textarea created through javascript is messed up

    
    

    
  
Comment on attachment 133648 [details] [diff] [review]
Proposed patch

I know I've run into this doubled layout frames problem before with
document.write, but I don't remember bug numbers or testcases.

        Attachment #133648 -
        Flags: review?(peterv) → review+

    
    

    
  
Yeah, I checked out those I could find like that and they look fine with this
patch...

Patch and regression test checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: