ASSERTION: 'bad height: 'metrics.height>=0' and ASSERTION: bad width: 'metrics.width>=0' with mspace

RESOLVED FIXED in mozilla15

Status

()

Core
MathML
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: fredw, Assigned: fredw)

Tracking

Trunk
mozilla15
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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.
Depends on: 411227
(Assignee)

Comment 2

6 years ago
Created attachment 587421 [details] [diff] [review]
crashtest

To apply after attachment 587416 [details] [diff] [review]...
(Assignee)

Comment 3

6 years ago
Created attachment 587423 [details]
testcase
(Assignee)

Comment 4

6 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → fred.wang
(Assignee)

Comment 5

6 years ago
Created attachment 587736 [details] [diff] [review]
Patch V1
Attachment #587736 - Flags: review?(karlt)
(Assignee)

Updated

6 years ago
Attachment #587421 - Flags: review?(karlt)
Attachment #587421 - Flags: review?(karlt) → review+
Comment on attachment 587736 [details] [diff] [review]
Patch V1

Perhaps mention in one of the comments that nsLineLayout doesn't expect negative widths.
Attachment #587736 - Flags: review?(karlt) → review+
(Assignee)

Updated

6 years ago
Blocks: 717546
(Assignee)

Updated

6 years ago
No longer blocks: 717546
Depends on: 717546
(Assignee)

Updated

6 years ago
Blocks: 717546
No longer depends on: 717546
(Assignee)

Comment 7

6 years ago
Created attachment 587991 [details] [diff] [review]
Patch V2
Attachment #587736 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Whiteboard: [good first bug]
(Assignee)

Updated

5 years ago
Keywords: helpwanted
(Assignee)

Comment 8

5 years ago
Created attachment 624503 [details] [diff] [review]
Patch V3
Attachment #587991 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=ec77d578bc5b
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/1213ffc89007
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa0630695d1b
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/1213ffc89007
https://hg.mozilla.org/mozilla-central/rev/aa0630695d1b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.