Closed Bug 1190646 Opened 9 years ago Closed 9 years ago

"ASSERTION: Stretching should be in the vertical direction"

Categories

(Core :: MathML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox42 --- affected
firefox43 --- fixed

People

(Reporter: jruderman, Assigned: jkitch)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(3 files, 1 obsolete file)

Attached file testcase
###!!! ASSERTION: Stretching should be in the vertical direction: 'isVertical', file layout/mathml/nsMathMLChar.cpp, line 1131

The assertion is part of code added in https://hg.mozilla.org/mozilla-central/rev/7d09c6ea9ae5 (patch for bug 407059).

Stir DOM found this by modifying https://dxr.mozilla.org/mozilla-central/source/layout/reftests/mathml/stretchy-largeop-1.html (test for bug 1092053).
Attached file stack
The code later checks if largeops have reached a sufficient size by checking the vertical direction only
https://hg.mozilla.org/mozilla-central/annotate/2ddec2dedced/layout/mathml/nsMathMLChar.cpp#l1240

so at first glance simply forcing isVertical to true for largeops is enough.

I'll have a more detailed look to confirm the correctness of that thought.
Attached patch patch (obsolete) — Splinter Review
The rest of the function defines the minimum size of all largeops as a height (not a problem as horizontally stretching laregops do not exist), so the result of this failed assertion is that largeops are not stretched to the appropriate size.

There are two options.  
1.  Change the callers so largeops are detected and are always passed vertical directions for stretching.
2.  Ignore the direction passed to the function and always treat largeops as vertical.

As there are no horizontally stretching largeops, option 2 seems the simpler choice.  The former is complicated by multiple code paths (with/without inferred mrows) and nesting.
Assignee: nobody → jkitch.bug
Status: NEW → ASSIGNED
Attachment #8644933 - Flags: review?(fred.wang)
I think the point of the assertion is really to verify that largeops are stretched in vertical direction, so it would be better to ensure the stretching is called with the right direction.

So I propose to check that all largeop operators in layout/mathml/mathfont.properties also have direction:vertical. I think it is enough to fix the bug?

Also, can you please open a bug for the XXX comment in

http://hg.mozilla.org/mozilla-central/file/3e51753a099f/layout/mathml/nsMathMLmoFrame.cpp#l455

and reference it in the source code ; with a warning about this stretch direction requirement?
No, I only use reftests/crashtests as starting points for DOMFuzz. I suppose I could add other static-ish test suites, or even try my luck with the DOM at the end of each mochitest.
Flags: needinfo?(jruderman)
Attachment #8644933 - Flags: review?(fred.wang)
Blocks: 1197771
Attached patch patchSplinter Review
All largeops are considered vertical in the operator dictionary.

A testcase covering the affected operators has been added to detect the assertion and ensure that it doesn't cause unintended stretching.
Attachment #8644933 - Attachment is obsolete: true
Attachment #8651766 - Flags: review?(fred.wang)
Comment on attachment 8651766 [details] [diff] [review]
patch

Review of attachment 8651766 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8651766 - Flags: review?(fred.wang) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/64f4fe2fb89b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: