Bug 160190

Summary:

Move parsing of boolean operator properties into MathMLOperatorElement

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:

alex, bfulgham, buildbot, commit-queue, darin, dbarton, esprehn+autocc, glenn, kondapallykalyan, rego, rniwa

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

160034, 160239, 160241

Bug Blocks:

156537, 160301

Attachments:

Description	Flags
Patch	buildbot: commit-queue-
Archive of layout-test-results from ews107 for mac-yosemite-wk2	none
Archive of layout-test-results from ews102 for mac-yosemite	none
Archive of layout-test-results from ews121 for ios-simulator-wk2	none
Archive of layout-test-results from ews113 for mac-yosemite	none
WIP Patch	none
Patch (applies after bugs 160241, 160245 and 160239)	none
Patch	darin: review+, buildbot: commit-queue-
Archive of layout-test-results from ews123 for ios-simulator-wk2	none

Frédéric Wang (:fredw)

Reported 2016-07-26 03:00:21 PDT

This is again one small step for bug 156537, to handle the following attributes: - fence - separator - stretchy - symmetric - largeop - movablelimits - accent We can and probably should also handle the determination of the default dictionary values in MathMLOperatorElement but anonymous operators created by the mfenced element make this a bit tedious and we also have to take the operator form into account so I'll consider that in follow-up patches.

Attachments
Patch (9.86 KB, patch) 2016-07-26 03:18 PDT, Frédéric Wang (:fredw)	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.11 MB, application/zip) 2016-07-26 04:03 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews102 for mac-yosemite (1.17 MB, application/zip) 2016-07-26 04:04 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (1.36 MB, application/zip) 2016-07-26 04:15 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews113 for mac-yosemite (1.82 MB, application/zip) 2016-07-26 04:23 PDT, Build Bot	no flags	Details
WIP Patch (7.40 KB, patch) 2016-07-28 02:59 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (applies after bugs 160241, 160245 and 160239) (10.36 KB, patch) 2016-07-28 06:44 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (15.68 KB, patch) 2016-08-02 05:48 PDT, Frédéric Wang (:fredw)	darin: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews123 for ios-simulator-wk2 (737.84 KB, application/zip) 2016-08-02 06:46 PDT, Build Bot	no flags	Details
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Frédéric Wang (:fredw)

Comment 1 2016-07-26 03:18:00 PDT

Created attachment 284570 [details] Patch

Build Bot

Comment 2 2016-07-26 04:03:55 PDT

Comment on attachment 284570 [details] Patch Attachment 284570 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1755711 New failing tests: mathml/opentype/large-operators-munderover.html mathml/presentation/stretchy-depth-height-symmetric.html imported/mathml-in-html5/mathml/relations/css-styling/displaystyle-1.html mathml/presentation/mo-whitespaces.html mathml/presentation/mo-movablelimits.html mathml/opentype/opentype-stretchy-horizontal.html mathml/presentation/inferred-mrow-stretchy.html mathml/presentation/mo-stretch.html mathml/presentation/stretchy-depth-height.html mathml/presentation/displaystyle-3.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4.html mathml/opentype/opentype-stretchy.html accessibility/math-mo-fences-and-separators.html mathml/presentation/displaystyle-1.html mathml/presentation/fenced.html mathml/presentation/fenced-mi.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2.html imported/mathml-in-html5/mathml/presentation-markup/operators/mo-axis-height-1.html mathml/presentation/attributes-accent-accentunder-dynamic.html mathml/presentation/mo-stretchy-vertical-bar.html mathml/presentation/mo-movablelimits-dynamic.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1.html mathml/presentation/displaystyle-2.html

Build Bot

Comment 3 2016-07-26 04:03:59 PDT

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

Build Bot

Comment 4 2016-07-26 04:04:38 PDT

Comment on attachment 284570 [details] Patch Attachment 284570 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1755695 New failing tests: mathml/opentype/large-operators-munderover.html mathml/presentation/stretchy-depth-height-symmetric.html imported/mathml-in-html5/mathml/relations/css-styling/displaystyle-1.html mathml/presentation/mo-whitespaces.html mathml/presentation/mo-movablelimits.html mathml/opentype/opentype-stretchy-horizontal.html mathml/presentation/inferred-mrow-stretchy.html mathml/presentation/mo-stretch.html mathml/presentation/stretchy-depth-height.html mathml/presentation/displaystyle-3.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4.html mathml/opentype/opentype-stretchy.html accessibility/math-mo-fences-and-separators.html mathml/presentation/displaystyle-1.html mathml/presentation/fenced.html mathml/presentation/fenced-mi.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2.html imported/mathml-in-html5/mathml/presentation-markup/operators/mo-axis-height-1.html mathml/presentation/attributes-accent-accentunder-dynamic.html mathml/presentation/mo-stretchy-vertical-bar.html mathml/presentation/mo-movablelimits-dynamic.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1.html mathml/presentation/displaystyle-2.html

Build Bot

Comment 5 2016-07-26 04:04:43 PDT

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

Build Bot

Comment 6 2016-07-26 04:15:41 PDT

Comment on attachment 284570 [details] Patch Attachment 284570 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1755700 New failing tests: mathml/opentype/large-operators-munderover.html mathml/presentation/stretchy-depth-height-symmetric.html imported/mathml-in-html5/mathml/relations/css-styling/displaystyle-1.html mathml/presentation/mo-whitespaces.html mathml/presentation/mo-movablelimits.html mathml/opentype/opentype-stretchy-horizontal.html mathml/presentation/inferred-mrow-stretchy.html mathml/presentation/mo-stretch.html mathml/presentation/stretchy-depth-height.html mathml/presentation/displaystyle-3.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4.html mathml/opentype/opentype-stretchy.html mathml/presentation/displaystyle-1.html mathml/presentation/fenced.html mathml/presentation/fenced-mi.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2.html imported/mathml-in-html5/mathml/presentation-markup/operators/mo-axis-height-1.html mathml/presentation/attributes-accent-accentunder-dynamic.html mathml/presentation/mo-stretchy-vertical-bar.html mathml/presentation/mo-movablelimits-dynamic.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1.html mathml/presentation/displaystyle-2.html

Build Bot

Comment 7 2016-07-26 04:15:45 PDT

Created attachment 284575 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.5

Build Bot

Comment 8 2016-07-26 04:23:54 PDT

Comment on attachment 284570 [details] Patch Attachment 284570 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1755720 New failing tests: mathml/opentype/large-operators-munderover.html mathml/presentation/stretchy-depth-height-symmetric.html imported/mathml-in-html5/mathml/relations/css-styling/displaystyle-1.html mathml/presentation/mo-whitespaces.html mathml/presentation/mo-movablelimits.html mathml/opentype/opentype-stretchy-horizontal.html mathml/presentation/inferred-mrow-stretchy.html mathml/presentation/mo-stretch.html mathml/presentation/stretchy-depth-height.html mathml/presentation/displaystyle-3.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4.html mathml/opentype/opentype-stretchy.html accessibility/math-mo-fences-and-separators.html mathml/presentation/displaystyle-1.html mathml/presentation/fenced.html mathml/presentation/fenced-mi.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2.html imported/mathml-in-html5/mathml/presentation-markup/operators/mo-axis-height-1.html mathml/presentation/attributes-accent-accentunder-dynamic.html mathml/presentation/mo-stretchy-vertical-bar.html mathml/presentation/mo-movablelimits-dynamic.html imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1.html mathml/presentation/displaystyle-2.html

Build Bot

Comment 9 2016-07-26 04:23:58 PDT

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

Frédéric Wang (:fredw)

Comment 10 2016-07-27 04:48:35 PDT

This patch won't work because the default operator values must also be calculated in MathMLOperatorElement first (or otherwise have a third state "default" for each flag).

Frédéric Wang (:fredw)

Comment 11 2016-07-28 02:59:23 PDT

Created attachment 284765 [details] WIP Patch

Frédéric Wang (:fredw)

Comment 12 2016-07-28 06:44:38 PDT

Created attachment 284780 [details] Patch (applies after bugs 160241, 160245 and 160239)

Frédéric Wang (:fredw)

Comment 13 2016-08-02 05:48:40 PDT

Created attachment 285102 [details] Patch

Build Bot

Comment 14 2016-08-02 06:46:27 PDT

Comment on attachment 285102 [details] Patch Attachment 285102 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1798467 New failing tests: media/track/media-element-enqueue-event-crash.html

Build Bot

Comment 15 2016-08-02 06:46:31 PDT

Created attachment 285105 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.5

Darin Adler

Comment 16 2016-08-02 09:16:32 PDT

Comment on attachment 285102 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285102&action=review > Source/WebCore/mathml/MathMLOperatorElement.cpp:159 > +void MathMLOperatorElement::parseProperty(MathMLOperatorDictionary::Flag flag) > +{ > + ASSERT(m_properties.dirtyFlags & flag); > + > + switch (flag) { > + case Accent: > + setOperatorFlagFromAttribute(flag, accentAttr); > + break; > + case Fence: > + setOperatorFlagFromAttribute(flag, fenceAttr); > + break; > + case LargeOp: > + setOperatorFlagFromAttribute(flag, largeopAttr); > + break; > + case MovableLimits: > + setOperatorFlagFromAttribute(flag, movablelimitsAttr); > + break; > + case Separator: > + setOperatorFlagFromAttribute(flag, separatorAttr); > + break; > + case Stretchy: > + setOperatorFlagFromAttribute(flag, stretchyAttr); > + break; > + case Symmetric: > + setOperatorFlagFromAttribute(flag, symmetricAttr); > + break; > + } > +} Two more thoughts about this: 1) Could separate out a function (inline if necessary) that maps a flag to an attribute name, rather than mixing that in with the algorithm. 2) Would be nice to have an assertion if the flag is not any of the valid values. The typical design for that is to use return and ASSERT_NOT_REACHED. It’s probably clean to combine that with suggestion (1) above since that function would naturally be done with a switch and a return inside each case, so ASSERT_NOT_REACHED after the switch would be a natural thing to do. > Source/WebCore/mathml/MathMLOperatorElement.cpp:212 > + } else if (name == accentAttr) > + m_properties.dirtyFlags |= Accent; > + else if (name == fenceAttr) > + m_properties.dirtyFlags |= Fence; > + else if (name == largeopAttr) > + m_properties.dirtyFlags |= LargeOp; > + else if (name == movablelimitsAttr) > + m_properties.dirtyFlags |= MovableLimits; > + else if (name == separatorAttr) > + m_properties.dirtyFlags |= Separator; > + else if (name == stretchyAttr) > + m_properties.dirtyFlags |= Stretchy; > + else if (name == symmetricAttr) > + m_properties.dirtyFlags |= Symmetric; Could imagine here using a function that maps an attribute name to a flag and uses a boolean or Optional to indicate that the value is not one of the flag attributes. Could be cleaner than repeating the list of the attribute and flags here.

Frédéric Wang (:fredw)

Comment 17 2016-08-02 11:48:41 PDT

Committed r204032: <http://trac.webkit.org/changeset/204032>

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