Closed Bug 220930 Opened 21 years ago Closed 21 years ago

[FIX]inspecting getElementsByTagName().length while a page is loading has strange consequences

Categories

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

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.6alpha

People

(Reporter: jsp, Assigned: bzbarsky)

Details

Attachments

(3 files, 2 obsolete files)

Same testcase showing a bit more information
2.08 KB, text/html
Details
Same as XML (with a few minor changes to make it valid XML)
2.12 KB, text/xml
Details
Patch with those comments addressed
5.12 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20030925
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20030925

Inspecting the length of a list returned by getElementsByTagName() early in page
loading causes incorrect lengths to be returned by later calls.

Reproducible: Always

Steps to Reproduce:
1. Load the test case.
2. Follow the directions in the page.
Actual Results:  
The number of items in the element list varies depending on whether or not the
length of a list of elements in the document is inspected while the document is
loading.

Expected Results:  
The length of the element list should always be the same.
Attached file test case (obsolete) — 

    
    

    
  


  

        Attachment #132465 -
        Attachment is obsolete: true

    
    

    
  
In HTML, it looks like the <body> is getting lost.  In XML, the <body> and
everything in it is getting lost.

This may be due to what the content model looks like when we run the script and
how that interacts with the document-observer behavior of content lists... I'll
try to look into this for 1.6b, but if someone has time before that and can work
on this, that would be much appreciated.
Assignee: dom_bugs → bzbarsky
Severity: normal → major
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.6beta

    
    

    
  
ccing some people who should know what's going on here...

    
    

    
  


  
The problem with HTML seems to be that when the <body> tag is opened the sink
does NOT send out notifications for the content that's been appended into the
document up to that point.  In particular, in the testcase given it never sends
a notification for the <body> node.  If the <script> is moved to be before the
<meta> and <title> tags, then those don't get notified about either. The upshot
of all this is that the content list has no way to find out that the <body>
node exists (since it relies on observer notifications after its length has
been gotten).

Not notifying document observers at all just because presshells happen to not
care yet (because they have not done an initial reflow) is just wrong.	The
patch moves that part of the logic down to the one observer that cares -- the
presshell.

The XML case is simpler, since the XML sink is not incremental (bug 18333). 
The problem there, as far as I can tell, is that the XML sink never sends out
any ContentAppended or ContentInserted notifications whatsoever.  I'm actually
somewhat surprised we render XML documents.  ;)  So I added a ContentInserted
on the root element just before the StartLayout() call (the idea being that
since we've not called StartLayout() the presshell will ignore the
ContentInserted per change above and won't have bogus frame construction).

The changes look pretty reasonable to me; the question is what people think
they may break... what should I test?

    
    

    
  
This is very much alpha material... ;)
Summary: inspecting getElementsByTagName().length while a page is loading has strange consequences → [FIX]inspecting getElementsByTagName().length while a page is loading has strange consequences
Target Milestone: mozilla1.6beta → mozilla1.6alpha

    
    

    
  
Comment on attachment 132857 [details] [diff] [review]
This fixes things, but needs lots of testing

heikki, Peter, do you have time to review this?

        Attachment #132857 -
        Flags: superreview?(hjtoi-bugzilla)

        Attachment #132857 -
        Flags: review?(peterv)

    
    

    
  
Comment on attachment 132857 [details] [diff] [review]
This fixes things, but needs lots of testing

heikki won't get to this for a while, but he said that in general it looks like
the right thing to do...

I've tested this on the various content-duplication bugs I've found in
bugzilla, and it's not breaking anything that's not already broken.

        Attachment #132857 -
        Flags: superreview?(hjtoi-bugzilla) → superreview?(jst)

    
    

    
  
Comment on attachment 132857 [details] [diff] [review]
This fixes things, but needs lots of testing

Seems like the right thing to do to me. sr=jst

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

    
    

    
  
Comment on attachment 132857 [details] [diff] [review]
This fixes things, but needs lots of testing

Looks good to me, though I think we want the same notification to happen in
nsXMLContentSink::OnTransformDone, just before the call to StartLayout.

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

    
    

    
  


  

        Attachment #132857 -
        Attachment is obsolete: true

    
    

    
  
Checked in.  Amazingly enough, Tp is not affected in any way.  :)
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

    
    

    
  
this appears to have broken xml pretty printing.  see bug 222539.

    
  
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: