348709 - "ASSERTION: non MathML cell frame" adding text node to <mtr>

Status: RESOLVED WORKSFORME

(Core :: MathML, defect)

Description

[Jesse Ruderman]

###!!! ASSERTION: non MathML cell frame: 'cellFrame->IsFrameOfType(nsIFrame::eMathML)', file /Users/admin/trunk/mozilla/layout/mathml/base/src/nsMathMLmtableFrame.cpp, line 460

This assertion was added in bug 348153.

This assertion might be related to a hang I'm seeing.

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[rbs]

The assertion is harmless in this particular case because, as presently implemented with the patch in bug 348153, the MathML table creator only kicks in if the element is in the MathML namespace:

+    nsTableCreator theTableCreator(aState.mPresShell);
+    nsTableCreator* tableCreator = &theTableCreator;
+#ifdef MOZ_MATHML
+    nsMathMLmtableCreator mtableCreator(aState.mPresShell);
+    if (aNameSpaceID == kNameSpaceID_MathML)
+       tableCreator = &mtableCreator;
+#endif

==================
Whereas with the script in the testcase:
  document.getElementById("mtr").appendChild(document.createTextNode("a"));
the textnode isn't in the MathML namespace. Because the textnode is actually causing pseudo-frames to be generated, I don't think it is necessary to try to be too clever here (such as testing the parent, etc).

Also it is possible to trigger the assertion by forcing a different namespace on <mtd> (while still keeping it with display:table-cell). So the assertion is not fool proof.

I am going to WONTFIX this -- unless you think that the assertion is harming your testing -- in which case I might just turn it into an NS_WARNING. 

(There is another more critical assertion that tests for display:table-cell irrespective of being a MathML subclass, and this is the one that should never fire:
+        DEBUG_VERIFY_THAT_FRAME_IS(cellFrame, TABLE_CELL);
)

Comment 3

[Jesse Ruderman]

Wontfixing this would be wrong, see bug 336074 comment 4.

Turning the assertion into a warning would be sufficient for fixing this bug, but it would be better if you could change the check so it doesn't assert in this case.  Then I'd be able to tell you if it fires in other cases, which would probably indicate a bug.  If you turn it into a warning and don't fix it to not fire in this case, I'll have to ignore it when I'm testing.

Comment 4

[rbs]

Attachment: patch - don't fire if it is not <mtd>

I changed the assrtion to fire only if <mtd> is given a non MathML frame. Are you happy with this?

Comment 5

[Jesse Ruderman]

WFM on trunk.  Is rbs's patch correct and still useful?

Comment 6

[Jesse Ruderman]

Crashtest checked in.