Bug 156542

Summary:

Ensure that RenderMathMLOperator::stretchTo functions are called with stretchy operators that have the correct direction

Product:

WebKit

Reporter:

Frédéric Wang (:fredw) <fred.wang>

Component:

MathML

Assignee:

Frédéric Wang (:fredw) <fred.wang>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cfleizach, commit-queue, darin, dbarton, esprehn+autocc, glenn, kondapallykalyan, mrobinson, rego

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

152244

Attachments:

Description	Flags
Patch	buildbot: commit-queue-
Archive of layout-test-results from ews112 for mac-yosemite	none
Patch	none
Patch	darin: review+

Frédéric Wang (:fredw)

Reported 2016-04-13 09:26:00 PDT

I'm extracting this change from bug 152244. This has already been done for horizontal stretchy operators in the RenderMathMLUnderOver refactoring (http://trac.webkit.org/changeset/199293) Let's do that for RenderMathMLRow and vertical stretchy operators too. We can then add appropriate ASSERT in the RenderMathMLOperator::stretchTo functions. Note that this change causes test failures for the following dynamic DOM/style tests, probably because some relayout that used to be forced by useless stretchTo called is not done anymore. mathml/presentation/mo-lspace-rspace-dynamic.html mathml/presentation/style-changed.html So the solution from bug 152244 was to be a bit more aggressive with updates, until we better manage such things in the MathML code (bug 156536).

Attachments
Patch (13.83 KB, patch) 2016-04-13 09:53 PDT, Frédéric Wang (:fredw)	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews112 for mac-yosemite (897.29 KB, application/zip) 2016-04-13 10:57 PDT, Build Bot	no flags	Details
Patch (12.93 KB, patch) 2016-04-14 00:33 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (13.81 KB, patch) 2016-04-14 04:37 PDT, Frédéric Wang (:fredw)	darin: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Frédéric Wang (:fredw)

Comment 1 2016-04-13 09:53:57 PDT

Created attachment 276329 [details] Patch

Build Bot

Comment 2 2016-04-13 10:57:12 PDT

Comment on attachment 276329 [details] Patch Attachment 276329 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1149342 New failing tests: fast/regions/mathml-crash.html

Build Bot

Comment 3 2016-04-13 10:57:15 PDT

Created attachment 276334 [details] Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5

Frédéric Wang (:fredw)

Comment 4 2016-04-14 00:33:40 PDT

Created attachment 276376 [details] Patch

Manuel Rego Casasnovas

Comment 5 2016-04-14 03:47:06 PDT

Comment on attachment 276376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276376&action=review LGTM. Just a small nit comment in the tests. > LayoutTests/mathml/presentation/row-nonstretchy-or-horizontal.html:2 > + Probably this is common in MathML tests, but its' not that common in other layout tests in WebKit. This kind of test description is usually in a <p> or something like that. So you can read them if you just run the test in browser. > LayoutTests/mathml/presentation/underover-nonstretchy-or-vertical.html:2 > + Ditto.

Frédéric Wang (:fredw)

Comment 6 2016-04-14 04:37:27 PDT

Created attachment 276387 [details] Patch

Darin Adler

Comment 7 2016-04-14 09:44:21 PDT

Comment on attachment 276387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276387&action=review > Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp:208 > + ASSERT(hasOperatorFlag(MathMLOperatorDictionary::Stretchy) && m_isVertical); One of our rules for assertions is that we don’t use && at the top level of an assert expression. Instead we separately assert the two expressions on either side of the &&. That way if the assertion fails we can quickly see which failed.

Frédéric Wang (:fredw)

Comment 8 2016-04-14 11:04:41 PDT

Committed r199544: <http://trac.webkit.org/changeset/199544>

Frédéric Wang (:fredw)

Comment 9 2016-04-14 11:05:55 PDT

(In reply to comment #7) > an assert expression. Instead we separately assert the two expressions on > either side of the &&. That way if the assertion fails we can quickly see > which failed. OK, I split them. Thanks.

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