399013 - Crash [@ UpdateViewsForTree] with popop stuff onoverflow/onunderflow and changing styles

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes after reload (happens automatically).

This seems to have regressed somehow between 2007-07-04 and 2007-07-05:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-07-04+04&maxdate=2007-07-05+09&cvsroot=%2Fcvsroot
I guess somehow a regression from bug 279703.

http://crash-stats.mozilla.com/report/index/6ff31651-75ae-11dc-8e53-001a4bd43ef6
0  	UpdateViewsForTree  	 mozilla/layout/base/nsCSSFrameConstructor.cpp:9625
1 	DoApplyRenderingChangeToTree 	mozilla/layout/base/nsCSSFrameConstructor.cpp:9657
2 	ApplyRenderingChangeToTree 	mozilla/layout/base/nsCSSFrameConstructor.cpp:9705
3 	nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList&) 	mozilla/layout/base/nsCSSFrameConstructor.cpp:9918
4 	nsCSSFrameConstructor::ProcessOneRestyle(nsIContent*, nsReStyleHint, nsChangeHint) 	mozilla/layout/base/nsCSSFrameConstructor.cpp:13008
5 	nsCSSFrameConstructor::ProcessPendingRestyles() 	mozilla/layout/base/nsCSSFrameConstructor.cpp:13054
6 	PresShell::DoFlushPendingNotifications(mozFlushType, int) 	mozilla/layout/base/nsPresShell.cpp:4443
7 	PresShell::FlushPendingNotifications(mozFlushType) 	mozilla/layout/base/nsPresShell.cpp:4407
8 	nsCSSFrameConstructor::RestyleEvent::Run() 	mozilla/layout/base/nsCSSFrameConstructor.cpp:13110
9 	nsThread::ProcessNextEvent(int, int*) 	mozilla/xpcom/threads/nsThread.cpp:490
10 	NS_ProcessNextEvent_P(nsIThread*, int) 	nsThreadUtils.cpp:227
11 	nsBaseAppShell::Run() 	mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:154
12 	nsAppStartup::Run() 	mozilla/toolkit/components/startup/src/nsAppStartup.cpp:170
13 	XRE_main 	mozilla/toolkit/xre/nsAppRunner.cpp:3142
14 	main 	mozilla/browser/app/nsBrowserApp.cpp:153
15 	WinMain 	mozilla/browser/app/nsBrowserApp.cpp:166
16 	__tmainCRTStartup 	crtexe.c:589

Comment 1

[Neil Deakin]

A simpler testcase has:

<menulist>
  <panel/>
  <menupopup id="a"/>
</menulist>

document.getElementById('a').setAttribute('style', 'display: block;');

The menulist initially has the <panel> in its popupList. The menupopup is the first child frame of the menulist.

When the menupopup is changed to use a block frame, the menulist's childlist is emptied, and the new block frame is appended as a sibling of the <panel>'s frame. That is, it is incorrectly placed as a sibling of the <panel> frame rather than as the first child of the menulist.

I think this is caused because nsCSSFrameConstructor::FindPreviousSibling doesn't account for the popup frames being in a different list.

Comment 2

[Neil Deakin]

Attachment: skip popups in the popup list

Comment 3

[Boris Zbarsky [:bzbarsky]]

Does the patch in bug 386642 fix this?  Or does the popup logic need to be exactly what it is in this patch?

Comment 4

[Neil Deakin]

Comment on attachment 284628 [details] [diff] [review]
skip popups in the popup list

Hmmm. I thought this bug sounded familiar. bz, your patch does indeed fix this bug, so let's ignore the patch here.

Comment 5

[Bruno 'Aqualon' Escherl]

FYI, I can't reproduce the crash from the testcase anymore with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102105 Minefield/3.0a9pre

Comment 6

[AndrewM]

I cannot reproduce either on Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007102105 Minefield/3.0a9pre ID:2007102105

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed by checkin for bug 386642, right?

Comment 8

[Martijn Wargers (dead)]

Right.
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007110805 Minefield/3.0b2pre

Comment 9

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b70a8533fbe

Comment 10

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/8b70a8533fbe