Closed Bug 220930 Opened 18 years ago Closed 18 years ago
Elements By Tag Name().length while a page is loading has strange consequences
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.
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?
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+
Checked in. Amazingly enough, Tp is not affected in any way. :)
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
this appears to have broken xml pretty printing. see bug 222539.
You need to log in before you can comment on or make changes to this bug.