Closed Bug 200347 Opened 21 years ago Closed 21 years ago

browser crashes on fieldset with position:fixed-style - Trunk M140B [@ nsSpaceManager::GetTranslation] [@ nsBlockBandData::Init]

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: theseer, Unassigned)

References

Details

(Keywords: crash, testcase, topcrash+, Whiteboard: landed1.4?)

Crash Data

Attachments

(3 files, 3 obsolete files)

patch for crash
1.81 KB, patch
Details | Diff | Splinter Review
AutoSpaceManager Patch v1.1
8.45 KB, patch
kinmoz
: review+
dbaron
: superreview+
asa
: approval1.4+
Details | Diff | Splinter Review
Testcase that does not bring out other fieldset bugs
455 bytes, text/html
Details 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4a) Gecko/20030401
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4a) Gecko/20030401

Even though the given testcase might be an abusive use of CSS and as of that to
be considered invalid html the browser crashes on loading this page.

Reproducible: Always

Steps to Reproduce:
1. load the testcase



Actual Results:  
crash

Expected Results:  
rendered the page - at least somehow ;)

<html>
 <body>
  <fieldset style="position:fixed;">
	<legend class="bblack14">Crash test</legend>
	hello world content
  </fieldset>
 </body>
</html>
###!!! ASSERTION: SpaceManager should be set in nsBlockReflowState:
'mSpaceManager', file nsBlockReflowState.cpp, line 90
Break: at file nsBlockReflowState.cpp, line 90
###!!! ASSERTION: null pointer: 'aSpaceManager', file nsBlockBandData.cpp, line
68
Break: at file nsBlockBandData.cpp, line 68

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 4274)]
0x4117cd0d in nsSpaceManager::GetTranslation (this=0x0, aX=@0xbfffe330,
    aY=@0xbfffe334) at ../../../base/src/nsSpaceManager.h:194
194       void GetTranslation(nscoord& aX, nscoord& aY) const { aX = mX; aY =
mY; }
(gdb) bt
#0  0x4117cd0d in nsSpaceManager::GetTranslation (this=0x0, aX=@0xbfffe330,
    aY=@0xbfffe334) at ../../../base/src/nsSpaceManager.h:194
#1  0x40da2782 in nsBlockBandData::Init (this=0xbfffe320, aSpaceManager=0x0,
    aSpace=@0xbfffe2d0) at nsBlockBandData.cpp:71
#2  0x40db1948 in nsBlockReflowState::nsBlockReflowState (this=0xbfffe2b0,
    aReflowState=@0xbfffe5d0, aPresContext=0x88ecdd0, aFrame=0x892bac0,
    aMetrics=@0xbfffe6c0, aBlockMarginRoot=0) at nsBlockReflowState.cpp:148
#3  0x40da4285 in nsBlockFrame::Reflow (this=0x892bac0,
    aPresContext=0x88ecdd0, aMetrics=@0xbfffe6c0, aReflowState=@0xbfffe5d0,
    aStatus=@0xbfffea5c) at nsBlockFrame.cpp:810
[...]
Keywords: crash, testcase
Summary: browser crashes on fieldset with position:fixed-style → browser crashes on fieldset with position:fixed-style [@ nsSpaceManager::GetTranslation ]
No dupes found, marking NEW.

more vars from GDB (I'm lost after frame 3):

(gdb) frame 1
#1  0x40da2782 in nsBlockBandData::Init (this=0xbfffe320, aSpaceManager=0x0,
    aSpace=@0xbfffe2d0) at nsBlockBandData.cpp:71
71        aSpaceManager->GetTranslation(mSpaceManagerX, mSpaceManagerY);
(gdb) p aSpaceManager
$9 = (nsSpaceManager *) 0x0
(gdb) frame 1
#1  0x40da2782 in nsBlockBandData::Init (this=0xbfffe320, aSpaceManager=0x0,
    aSpace=@0xbfffe2d0) at nsBlockBandData.cpp:71
71        aSpaceManager->GetTranslation(mSpaceManagerX, mSpaceManagerY);
(gdb) p mSpaceManagerX
$10 = 0
(gdb) p mSpaceManagerY
$11 = 0
(gdb) frame 2
#2  0x40db1948 in nsBlockReflowState::nsBlockReflowState (this=0xbfffe2b0,
    aReflowState=@0xbfffe5d0, aPresContext=0x88ecdd0, aFrame=0x892bac0,
    aMetrics=@0xbfffe6c0, aBlockMarginRoot=0) at nsBlockReflowState.cpp:148
148       mBand.Init(mSpaceManager, mContentArea);
(gdb) p aReflowState.mSpaceManager
$12 = (nsSpaceManager *) 0x0
(gdb) frame 3
#3  0x40da4285 in nsBlockFrame::Reflow (this=0x892bac0,
    aPresContext=0x88ecdd0, aMetrics=@0xbfffe6c0, aReflowState=@0xbfffe5d0,
    aStatus=@0xbfffea5c) at nsBlockFrame.cpp:810
810                                NS_BLOCK_MARGIN_ROOT & mState);
Assignee: asa → roc+moz
Status: UNCONFIRMED → NEW
Component: Browser-General → Layout: View Rendering
Ever confirmed: true
QA Contact: asa → ian
So the problem is that the fieldset frame does nothing with the
NS_BLOCK_SPACE_MGR flag that's set on it when it's created.  As in, it does not
create a space manager in its reflow method.  If it's fixed-positioned, there is
no space manager coming in in the reflow state... so there is no space manager
in the reflow state given to the legend frame.  Now the legend frame does _not_
have the space manager state bit set, so it never creates one, flows as a block,
and crashes.

Possible solutions:

1)  Set that bit on legend frames
2)  Make the fieldset frame respect that bit (depending on what the desired
    behavior is, here....)
Component: Layout: View Rendering → Layout: Form Controls
.
Assignee: roc+moz → form
QA Contact: ian → desale
Attached patch patch for crash — — Splinter Review 
This fixes the crash; dunno if this gives us the right behavior, though....
Attached file Testcase (obsolete) — 
I added a float and some more text to the div to test the effect of the space
manager.
Attached patch Patch for crash (obsolete) — — Splinter Review 
This fixes the crash too, more at its root, but the space manager created
doesn't seem to work--floats don't float, and I thought that wsa the purpose of
the space manager.  I wonder why fieldset does not simply inherit from div. 
That's all it is, a div with a funky border.
Oh yeah, I moved nsAutoSpaceManager from nsBlockFrame.cpp into
nsSpaceManager.h.
Attachment #120983 - Attachment is obsolete: true
OK, the floaters float when the fieldset has a fixed width, I bet the fact that
it was not floating in the other testcase is related to a bug on fixed width
unconstrained width fieldsets.
Attachment #120982 - Attachment is obsolete: true
Attachment #121007 - Attachment is obsolete: true
oops, that last testcase was the patch, not the testcase
Attachment #121000 - Flags: review?(kin)
Blocks: 145853
Comment on attachment 121000 [details] [diff] [review]
AutoSpaceManager Patch v1.1

So I'm wondering if we really want code in layout/base dependent on headers in
layout/html/base.

If the code for nsAutoSpaceManager is going to rely on nsHTMLReflowState,
should it be moved to nsHTMLContainerFrame instead? Or if you really want it to
live in the space manager code in layout/base, should it be made more generic?
That is, should nsAutoSpaceManager be modified to just manage an
nsSpaceManager** pointer to avoid sucking in nsHTMLReflowState.h since that's
all it really needs?
Well, nsHTMLReflowState is so basic, required for reflow, that I think "move
nsHTMLReflowState to layout/base" is more the appropriate action.  I think
nsAutoSpaceManager is really designed to work with a reflow state (its job is to
switch out space managers when a block requires a new space manager), so it is
appropriate to place it there.  I also think it really belongs in
nsSpaceManager, but I'd be willing to move it into nsHTMLReflowState (that's not
such a bad choice either) or its own file in layout/html/base if people so desire.
Crashes build 2003043008 on WinXP too, marking OS:all
OS: Linux → All
No target milestone?

I have seen this bug on two spanish webpages, so in the near future this bug
could be used to hurt mozilla users :-(
Somebody had the courtesy to mention this on Slashdot today, "in the interest 
of fairness", since /. posted a similar crasher for IE a couple of days ago.  
Anyway, expect all the L337 Hax0R wannabe kids to start sticking this on pages, 
trying to screw with Moz users...
Is this bug a regression of bug 130251 - they seem pretty similar.
*** Bug 204849 has been marked as a duplicate of this bug. ***
Kin, want to reconsider review?  HTMLReflowState should probably move to
layout/base, but I think that's out of scope.  Refactoring is also possible, or
moving it into its own file, though I prefer the first one.
Adding Trunk M140B and [@ nsBlockBandData::Init] to summary and topcrash+
keyword.  A lot of users have been crashing on a test page posted on SlashDot. 
Here is one set of crashes for M140B:

Count   Offset    Real Signature
[ 8   nsBlockBandData::Init edc92ffd - nsBlockBandData::Init ]
[ 2   nsBlockBandData::Init a6ce067a - nsBlockBandData::Init ]
[ 2   nsBlockBandData::Init 8230fba5 - nsBlockBandData::Init ]
[ 2   nsBlockBandData::Init 74914da7 - nsBlockBandData::Init ]
[ 1   nsBlockBandData::Init f6ede7d0 - nsBlockBandData::Init ]
[ 1   nsBlockBandData::Init f2f28dc2 - nsBlockBandData::Init ]
[ 1   nsBlockBandData::Init 957a816d - nsBlockBandData::Init ]
[ 1   nsBlockBandData::Init 7127f19e - nsBlockBandData::Init ]
[ 1   nsBlockBandData::Init 6bfb8d28 - nsBlockBandData::Init ]
 
     Crash date range: 2003-05-07 to 2003-05-11
     Min/Max Seconds since last crash: 15 - 44620
     Min/Max Runtime: 18 - 44620
 
     Count   Platform List 
     11   Windows NT 5.0 build 2195
     3   Windows NT 5.1 build 2600
     2   Windows NT 4.0 build 1381
     2   Windows 98 4.10 build 67766446
     1   Windows 98 4.90 build 73010104
 
     Count   Build Id List 
     19   2003050714
 
     No of Unique Users        17
 
 Stack trace(Frame) 

	 nsBlockBandData::Init
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockBandData.cpp  line 71] 
	 nsBlockReflowState::nsBlockReflowState
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockReflowState.cpp  line 151] 
	 nsBlockFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp  line 821] 
	 nsLegendFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/forms/src/nsLegendFrame.cpp  line 127] 
	 nsContainerFrame::ReflowChild
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsContainerFrame.cpp  line 973] 
	 nsFieldSetFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/forms/src/nsFieldSetFrame.cpp  line 418] 
	 nsAbsoluteContainingBlock::ReflowAbsoluteFrame
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsAbsoluteContainingBlock.cpp
 line 508] 
	 nsAbsoluteContainingBlock::IncrementalReflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsAbsoluteContainingBlock.cpp
 line 341] 
	 ViewportFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsViewportFrame.cpp  line 314] 
	 IncrementalReflow::Dispatch
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp  line 916] 
	 PresShell::ProcessReflowCommands
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp  line 6664] 
	 ReflowEvent::HandleEvent
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp  line 6509] 
	 PL_HandleEvent	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c  line 660] 
	 PL_ProcessPendingEvents	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c 
line 596] 
	 _md_EventReceiverProc	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c 
line 1396] 
	 nsAppShellService::Run
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp  line 479] 
	 main1	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1284] 
	 main	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1650] 
	 WinMain	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1672] 
	 WinMainCRTStartup()  
	 KERNEL32.dll + 0x2847c (0x77ea847c)   
 
     (19928566)	Comments: &lt;html&gt;  &lt;fieldset style="position:fixed;"&gt;
 &lt;legend&gt;Hybris&lt;/legend&gt;  &lt;/fieldset&gt;  &lt;/html&gt;
     (19915709)	Comments: Code posted on slashdot to crash a mozilla browser: 
&lt;html&gt;  &lt;fieldset style="position:fixed;"&gt; 
&lt;legend&gt;Crash&lt;/legend&gt;  &lt;/fieldset&gt;   &lt;/html&gt;  
     (19915562)	Comments: &lt;html&gt;  &lt;fieldset style="position:fixed;"&gt;
 &lt;legend&gt;Crash&lt;/leyend&gt;  &lt;/fieldset&gt;  &lt;/html&gt;
     (19910316)	Comments: Visiting a test page that claims to crash Mozilla. It
was right.
     (19888830)	URL: http://www.geeklife.com/files/crashMoz.html
     (19888830)	Comments: This is a 5-line HTML page which crashes Mozilla. 
This was listed in a Slashdot story:    
http://slashdot.org/article.pl?sid=03/05/05/2038222&mode=nested&tid=167&tid=99 
  This crashed the most recently downloaded Mozilla (1.4 beta).
     (19887663)	URL: http://www.geeklife.com/files/crashMoz.html

Since we have a patch ready and a easily reproducible testcase, I'm also
nominating this for blocking1.4? Any chance of getting this fix in for Mozilla
1.4 final?
Flags: blocking1.4?
Keywords: topcrash+
Summary: browser crashes on fieldset with position:fixed-style [@ nsSpaceManager::GetTranslation ] → browser crashes on fieldset with position:fixed-style - Trunk M140B [@ nsSpaceManager::GetTranslation] [@ nsBlockBandData::Init]
*** Bug 205409 has been marked as a duplicate of this bug. ***
Flags: blocking1.4? → blocking1.4+
Comment on attachment 121000 [details] [diff] [review]
AutoSpaceManager Patch v1.1

r=kin@netscape.com
Attachment #121000 - Flags: review?(kin) → review+
John, do we need another review on this? Can we get it landed soon?

--Asa
Attachment #121000 - Flags: superreview?(dbaron)
Attachment #121000 - Flags: superreview?(dbaron) → superreview+
Attachment #121000 - Flags: approval1.4?
Comment on attachment 121000 [details] [diff] [review]
AutoSpaceManager Patch v1.1

a=asa (on behalf of drivers) for checkin to 1.4
Attachment #121000 - Flags: approval1.4? → approval1.4+
Did this land yet? I don't see anything in bonsai.
Whiteboard: landed1.4?
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
*** Bug 207839 has been marked as a duplicate of this bug. ***
V.
Status: RESOLVED → VERIFIED
Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/afc662d52ab1
Flags: in-testsuite+
Crash Signature: [@ nsSpaceManager::GetTranslation] [@ nsBlockBandData::Init]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: