353897 - [FIX]Crash [@ nsMathMLContainerFrame::FixInterFrameSpacing] using position:fixed when position: static rule in mathml.css is removed

Status: RESOLVED FIXED

(Core :: MathML, defect, P1)

Description

[Martijn Wargers (dead)]

See upcoming testcase, which crashes current trunk build when the following rule in mathml.css is removed:
/* MathML doesn't permit positioning */
*, * > *|* {
  position: static !important;
}

Talkback ID: TB23627768X
nsMathMLContainerFrame::FixInterFrameSpacing  [mozilla\layout\mathml\base\src\nsmathmlcontainerframe.cpp, line 1408]
nsMathMLContainerFrame::FinalizeReflow  [mozilla\layout\mathml\base\src\nsmathmlcontainerframe.cpp, line 530]
nsMathMLContainerFrame::Reflow  [mozilla\layout\mathml\base\src\nsmathmlcontainerframe.cpp, line 1136]
nsAbsoluteContainingBlock::ReflowAbsoluteFrame  [mozilla\layout\generic\nsabsolutecontainingblock.cpp, line 563]

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

So the issue here is that 

1404    nsMathMLContainerFrame::FixInterFrameSpacing(nsHTMLReflowMetrics& aDesiredSize)
1405    {
1406      nscoord gap = 0;
1407      nsIContent* parentContent = mParent->GetContent();
1408      nsIAtom *parentTag = parentContent->Tag();

crashes because mParent is the viewport, so parentContent is null.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Disable floating and positioning of MathML frames.

This should do what we want.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Again, I'd like to land this on branches.

Comment 5

[rbs]

Comment on attachment 239922 [details] [diff] [review]
Disable floating and positioning of MathML frames.

I wouldn't care if <math> didn't float too, but it doesn't cost us much to have this isMath test.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 239922 [details] [diff] [review]
Disable floating and positioning of MathML frames.

Requesting branch approvals.  This is MathML-only and should be pretty safe -- we have CSS that _should_ mostly prevent us hitting these cases anyway, but if we do hit them all this patch does is disallow mathml-namespaced nodes other than <math> from being floated or absolutely positioned or fixed-positioned.

Comment 8

[Mike Schroepfer]

Comment on attachment 239922 [details] [diff] [review]
Disable floating and positioning of MathML frames.

Approved for RC2.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Branch version of patch

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed on the 1.8.1 branch.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 239922 [details] [diff] [review]
Disable floating and positioning of MathML frames.

approved for 1.8.0 branch, a=dveditz for drivers

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fixed in 1.8.0.8

Comment 13

[Carsten Book [:Tomcat]]

verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20061002 Firefox/2.0

Comment 14

[Marcia Knous [:marcia]]

verified fixed on the 1.8.0 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.8) Gecko/20061025 Firefox/1.5.0.8. I do not crash with the testcase in Comment 1. Adding keyword.