Last Comment Bug 716349 - ASSERTION: 'bad height: 'metrics.height>=0' and ASSERTION: bad width: 'metrics.width>=0' with mspace
: ASSERTION: 'bad height: 'metrics.height>=0' and ASSERTION: bad width: 'metric...
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Frédéric Wang (:fredw)
: Anthony Jones (:kentuckyfriedtakahe, :k17e)
Depends on: 411227
Blocks: 717546
  Show dependency treegraph
Reported: 2012-01-08 03:58 PST by Frédéric Wang (:fredw)
Modified: 2012-05-17 20:30 PDT (History)
4 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

crashtest (960 bytes, patch)
2012-01-10 12:12 PST, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review
testcase (194 bytes, text/html)
2012-01-10 12:13 PST, Frédéric Wang (:fredw)
no flags Details
Patch V1 (2.80 KB, patch)
2012-01-11 09:47 PST, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review
Patch V2 (2.87 KB, patch)
2012-01-12 03:52 PST, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Patch V3 (2.88 KB, patch)
2012-05-16 13:10 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review

Description Frédéric Wang (:fredw) 2012-01-08 03:58:06 PST
mspace allows the user to give arbitrary dimensions and thus it is easy to produce assertions with negative values. For example:

<mspace width="-10px"/>
<mspace height="-10px"/>

We should fix nsMathMLmspaceFrame::Reflow. Note that mspace is cited among the MathML elements that permit "negative spacing" so we will have a bit more to do than just forbidding negative attribute values.
Comment 1 Frédéric Wang (:fredw) 2012-01-08 04:05:09 PST
It will however maybe make sense to have the constraint "height >= 0 && width >= 0" rather than the less strict constraint "height + width >= 0". I hardly see how something like:

<mspace height="20px" depth="-10px"/> may be useful for a space.

In that case, the assertions for vertical metrics can be fixed by bug 411227, if we remove the nsMathMLElement::PARSE_ALLOW_NEGATIVE flag when parsing the values of the height/depth attributes.
Comment 2 Frédéric Wang (:fredw) 2012-01-10 12:12:16 PST
Created attachment 587421 [details] [diff] [review]

To apply after attachment 587416 [details] [diff] [review]...
Comment 3 Frédéric Wang (:fredw) 2012-01-10 12:13:04 PST
Created attachment 587423 [details]
Comment 4 Frédéric Wang (:fredw) 2012-01-11 05:27:56 PST
As said in comment 1, I suggest to forbid negative values for height and depth of mspace.

For mspace with negative width, I think we should modify nsMathMLmspaceFrame::Reflow to set

mBoundingMetrics.width = NS_MAX(0, mWidth);
aDesiredSize.width = NS_MAX(0, mWidth);

and this should fix this bug.

Then, if we want to implement negative spaces, we should modify nsMathMLContainerFrame::RowChildFrameIterator to handle mX in a specific manner when mChildFrame->GetContent()->Tag() == == nsGkAtoms::mspace_ with mChildFrame->mWidth < 0. However, I'm not sure it will work when the mspace is a direct child of the <math/> element.
Comment 5 Frédéric Wang (:fredw) 2012-01-11 09:47:10 PST
Created attachment 587736 [details] [diff] [review]
Patch V1
Comment 6 Karl Tomlinson (back Dec 13 :karlt) 2012-01-11 14:05:55 PST
Comment on attachment 587736 [details] [diff] [review]
Patch V1

Perhaps mention in one of the comments that nsLineLayout doesn't expect negative widths.
Comment 7 Frédéric Wang (:fredw) 2012-01-12 03:52:28 PST
Created attachment 587991 [details] [diff] [review]
Patch V2
Comment 8 Frédéric Wang (:fredw) 2012-05-16 13:10:42 PDT
Created attachment 624503 [details] [diff] [review]
Patch V3
Comment 9 Frédéric Wang (:fredw) 2012-05-17 00:16:22 PDT

Note You need to log in before you can comment on or make changes to this bug.