Bug 160190 – Move parsing of boolean operator properties into MathMLOperatorElement

• Component: MathML
• Status: RESOLVED FIXED

[Frédéric Wang (:fredw)]

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.

[Frédéric Wang (:fredw)]

Created attachment 284570 [details]
Patch

[Build Bot]

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]

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 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]

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 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]

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 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]

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)]

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)]

Created attachment 284765 [details]
WIP Patch

[Frédéric Wang (:fredw)]

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

[Frédéric Wang (:fredw)]

Created attachment 285102 [details]
Patch

[Build Bot]

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]

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 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)]

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