Closed Bug 625722 Opened 14 years ago Closed 13 years ago

Firefox freezes in Mozilla TreeWalker demo

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86_64
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- final+

People

(Reporter: pleugner, Assigned: craig.topper)

References

(
URL
)

Details

(Keywords: hang, regression)

Attachments

(1 file, 1 obsolete file)

Patch with test
2.73 KB, patch
sicking
: review+
sicking
: approval2.0+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20110103 Gentoo Firefox/3.6.13
Build Identifier: Firefox 40b8

Firefox freezes when pressing the "All attributes" button at http://www-archive.mozilla.org/docs/dom/samples/treewalkerdemo.xml.

I found this by investigating code in my addon that freezes on Firefox 4, which uses a TreeWalker as well, but above URL is easier to report.

The demo works in FF3

Reproducible: Always

Steps to Reproduce:
1. Go to http://www-archive.mozilla.org/docs/dom/samples/treewalkerdemo.xml
2. Press the "All attributes" button

Actual Results:  
Firefox freezes and has too be killed

Expected Results:  
Do not freeze and show me the attributes of the XML nodes
I can confirm the hang with Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110114 Firefox/4.0b10pre SeaMonkey/2.1b2pre

I will try to generate a stacktrace and probably a regression range later

Reporter: Do you know with which FF4 build/beta the hang started ?
Keywords: hang
OS: Linux → All
Sorry no, I just started to port my addon and started with Beta 8.
Status: UNCONFIRMED → NEW
Ever confirmed: true
nsTreeWalker::FirstChildInternal seems to take all the time.
Could this be a regression from Bug 577438?
blocking2.0: --- → ?
Last good nightly: 2010-07-21 First bad nightly: 2010-07-22

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1ac07fe5f6
c9&tochange=0f5fc40c6a0f

the checkin from Bug 577438 is in that timeframe !
Keywords: regressionwindow-wanted
Assignee: nobody → jonas
blocking2.0: ? → final+
This loop doesn't always update node and thus doesn't terminate. Haven't checked to see if this what caused this issue, but it's definitely broken. I'll write a patch and test soon.

        do {
            nsINode *sibling = aReversed ? node->GetPreviousSibling()
                                         : node->GetNextSibling();
            if (sibling) {
                node = sibling;
                break;
            }
            nsINode *parent = node->GetNodeParent();

            if (!parent || parent == mRoot || parent == mCurrentNode) {
                return NS_OK;
            }

        } while (node);
If it can help, here is a simple test case I wrote before I found that this bug seems to be already filled :
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
	<title>Bug treeWalker</title>
	<script>
		function filterNone(){
			return NodeFilter.FILTER_SKIP;
		}
		function test(){
			var vTw = document.createTreeWalker(document, NodeFilter.SHOW_ELEMENT, filterNone, false);
			vTw.firstChild();
			alert("Ok");
		}
	</script>
</head>
<body>
	<button onclick="test();">test</div>
</body>
</html>
Attached patch Patch (obsolete) — — Splinter Review 
This patches assigns node = parent just before the end of the loop so it will walk back up the tree like it was supposed to.
Assignee: jonas → craig.topper
Status: NEW → ASSIGNED
Attachment #504276 - Flags: review?(jonas)
Comment on attachment 504276 [details] [diff] [review]
Patch

This really needs some tests.
Attached patch Patch with test — — Splinter Review 

        Attachment #504276 -
        Attachment is obsolete: true

        Attachment #504322 -
        Flags: review?(jonas)

        Attachment #504276 -
        Flags: review?(jonas)

        Attachment #504322 -
        Flags: review?(jonas)

        Attachment #504322 -
        Flags: review+

        Attachment #504322 -
        Flags: approval2.0+

    
    

    
  
Checkin http://hg.mozilla.org/mozilla-central/rev/2b3bc2611e6e
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

    
    

    
  
It still dooesn't work right, but at least it doesn't hang anymore. If you go to http://www-archive.mozilla.org/docs/dom/samples/treewalkerdemo.xml and press the "All datatypes" button with FF Beta 10, you only get the first type node, while in FF 3 you get all the type nodes in the example XML.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  
Peter, could you file that issue as a new bug, since it isn't a hang which this
bug is about.

    
    

    
  
Ok, follow up in Bug 628938
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Component: DOM: Traversal-Range → DOM: Core & HTML

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: