See upcoming testcase. When following the steps in the testcase, I get a freeze/hang of Mozilla and I need to shut down Mozilla with the Task Manager.
Uhm, the steps to reproduce are specific for the Firefox DOM Inspector, but it should be quite easy to reproduce also in Mozilla's DOM Inspector (I've already verified that it freezes the browser also there).
WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050216
Maybe I was a bit unclear with step 2, it says: 2. Unfold the DOM tree till the body element. With that I mean that the body element also must be unfolded.
13 years ago
i don't suppose you have a stack/profile/explanation of what's going on?
Ctrl-C with gdb under mingw doesn't seem to work, so I can't interrupt the program.
Created attachment 220370 [details] [diff] [review] Partial patch This fixes two issues: 1) The ContentAppended handler was broken; with this patch we see all three of the <span> nodes in the tree. 2) inDOMView::GetParentIndex didn't deal well with not finding a parent. With this patch we still throw from inDOMView::GetParentIndex, while looking for the parent for a <span>. The parent pointer on said span looks pretty bogus to me... Martijn, do you want to try figuring out what's going on with that part? Or should I try to look into it?
(In reply to comment #7) > Martijn, do you want to try figuring out what's going on with that part? Or I wish I was able to do that. But my debugging skills are just too poor :(
Created attachment 221994 [details] [diff] [review] Patch I fixed an assert and a few other minor issues while I was here. The substantive fix for this patch is the fix to ContentAppended to handle more than one node and the fix to ContentInserted to properly deal with insertion of kids of nodes corresponding to closed rows. The change to ContentRemoved() to pass the right number to RowCountChanged() fixes an assert if an open row with kids is removed. The change to invalidate the parent if it's no longer a container hides the twisty on things all of whose kids get removed. The rest is minor safety cleanup.
Comment on attachment 221994 [details] [diff] [review] Patch Irrelevant side note: I wonder whether Expand/CollapseNode should return the number of rows changed (e.g. ToggleOpenState calls RowCountChanged(index + 1, node->isOpen ? CollapseNode(index) : ExpandNode(index));
re-fixed on the layout/inspector sources since this landed in the middle of bug 325100
I keep hitting this on 1.8.1 branch, while inspecting nodes on Google Maps (send e-mail link near upper right corner, inspecting the DOM nodes that are on the page); looks like this patch never landed there. Requesting blocking1.8.14 to see if we can get this in.
Requesting blocking is the wrong way to go about that. The right way is to take the patch, apply it to 1.8, fix any merge issues, test it to make sure it works, then post a diff against 1.8 and request approval to check it in. Or review before approval if you had to make changes to the patch. I'm not going to be doing the above, so if you want this fixed you'll need to find someone who will.