Closed Bug 347507 Opened 18 years ago Closed 18 years ago

MathML: negative rowspan causes "ASSERTION: internal error: 'originates'"

Categories

(Core :: MathML, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: jruderman)

References

Details

(Keywords: assertion, testcase)

Attachments

(2 files, 1 obsolete file)

###!!! ASSERTION: internal error: 'originates', file /Users/admin/trunk/mozilla/layout/mathml/base/src/nsMathMLmtableFrame.cpp, line 193
Attached file testcase
I think negative rowspans can cause crashes too.
Blocks: 347580
I am not hitting the assertion, BTW.

But it might require to ignore the attributes if they are < 0, in nsMathMLmtableFrame.cpp:

nsMathMLmtdFrame::GetRowSpan()
nsMathMLmtdFrame::GetColSpan()
MathML says "The rowspan and columnspan attributes allow a specific matrix element to be treated as if it occupied the number of rows or columns specified. The interpretation of how this larger element affects specifying subsequent rows and columns is meant to correspond with the similar attributes for HTML 3.2 tables."

HTML 3.2 says "rowspan: This takes a positive integer value specifying the number of rows spanned by this cell. It defaults to one."

HTML 4.01 says "rowspan: This attribute specifies the number of rows spanned by the current cell. The default value of this attribute is one ("1"). The value zero ("0") means that the cell spans all rows from the current row to the last row of the table section (THEAD, TBODY, or TFOOT) in which the cell is defined."

Since the value of zero is special in HTML 4.01 and not allowed in HTML 3.2, and the MathML spec says to refer to HTML 3.2, I think a rowspan of zero should be ignored in MathML.

There's also http://lxr.mozilla.org/seamonkey/search?string=MAX_ROWSPAN ...
Attached patch patch: ignore non-positive spans (obsolete) — Splinter Review
This fixes the assertion for me.  It also fixes a crash with a different testcase (not attached).

I'll file a separate bug for moving the MAX_ROWSPAN constant somewhere where this file can use it.
Assignee: rbs → jruderman
Status: NEW → ASSIGNED
Attachment #232514 - Flags: superreview?(roc)
Attachment #232514 - Flags: review?(rbs)
I will go for < 0. The MathML spec is an old spec pre-dating HTML 4, and it is now been updated. No need to be tied up to the past, with no way of emulating what the special 0 does.
What I pasted earlier was from the MathML 1 spec.  MathML 2 (http://www.w3.org/TR/MathML2/chapter3.html#presm.mtd) says:

"rowspan: positive-integer"

"The rowspan and columnspan attributes allow a specific matrix element to be treated as if it occupied the number of rows or columns specified. The interpretation of how this larger element affects specifying subsequent rows and columns is meant to correspond with the similar attributes for HTML 4.01 tables."

I don't think MathML 2 predates HTML 4, so they probably disallowed a value of 0 intentionally.

Do you know what MathML 3 will say?
I instead think that they may have overlooked the full power of 0, and that 0 has gone on to take a different meaning in HTML 4 (which is precisely what I am trying to say -- that the MathML spec will be adjusted if necessary to remain in sync).

Unfortunately, I do not have the cycles to be simultaneously maintaining the code, iterating the spec, etc, etc... But you might want to raise the matter to the spec guys.
Less of a change from previous Gecko behavior, and still fixes the assertion and crash.
Attachment #232514 - Attachment is obsolete: true
Attachment #232630 - Flags: superreview?(roc)
Attachment #232630 - Flags: review?(rbs)
Attachment #232514 - Flags: superreview?(roc)
Attachment #232514 - Flags: review?(rbs)
Comment on attachment 232630 [details] [diff] [review]
patch: ignore negative spans

r=rbs
Attachment #232630 - Flags: review?(rbs) → review+
Attachment #232630 - Flags: superreview?(roc) → superreview+
Checking in nsMathMLmtableFrame.cpp;
/cvsroot/mozilla/layout/mathml/base/src/nsMathMLmtableFrame.cpp,v  <--  nsMathMLmtableFrame.cpp
new revision: 1.54; previous revision: 1.53
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
> I'll file a separate bug for moving the MAX_ROWSPAN constant somewhere where
> this file can use it.

Bug 348954.
Crashtest checked in.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: