Commit b7f7d543 authored by Frédéric Wang's avatar Frédéric Wang Committed by Commit Bot

[mathml] Fix dictionary category for unknown operators with explicit form

MathMLOperatorElement::ComputeDictionaryCategory was wrongly implemented
in [1] and returns an unknown category for an operator that is not in
the dictionary AND has an explicit form attribute. This caused a crash
when the function was finally used in [2] and [3].

This CL fixes that mistake and adds a DCHECK after the unique call to
ComputeDictionaryCategory to ensure the category is not unknown.
Additionally, the documentation of the corresponding low-level
platform/text API is updated to make clear it never returns an unknown
category.

WPT tests are added, one of them contains an munderover whose base is an
operator outside the dictionary with an explicit form, allowing to
ensure no crash happens for the code added in [2] [3]. Another one
verifies default operator spacing and will cover the code added in [4].

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2368362
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2390760
[3] https://chromium-review.googlesource.com/c/chromium/src/+/2383023
[4] https://chromium-review.googlesource.com/c/chromium/src/+/2390652

Bug: 6606, 1124617
Change-Id: Ic6ba0b663d14634ca5c66070a5b3cfc2eaa3d198
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390633Reviewed-by: default avatarRob Buis <rbuis@igalia.com>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804559}
parent 30580eba
......@@ -189,9 +189,9 @@ void MathMLOperatorElement::ComputeDictionaryCategory() {
return;
}
}
// Step 4.
properties_.dictionary_category = MathMLOperatorDictionaryCategory::kNone;
}
// Step 4.
properties_.dictionary_category = MathMLOperatorDictionaryCategory::kNone;
}
}
......@@ -209,6 +209,8 @@ void MathMLOperatorElement::ComputeOperatorProperty(OperatorPropertyFlag flag) {
} else {
// By default, the value specified in the operator dictionary are used.
ComputeDictionaryCategory();
DCHECK(properties_.dictionary_category !=
MathMLOperatorDictionaryCategory::kUndefined);
if (MathMLOperatorDictionaryCategories
[std::underlying_type_t<MathMLOperatorDictionaryCategory>(
properties_.dictionary_category)]
......
......@@ -31,6 +31,7 @@ enum MathMLOperatorDictionaryForm { kInfix, kPrefix, kPostfix };
// FindCategory takes a UTF-16 string and form (infix, prefix, postfix) as input
// and returns the operator dictionary category for this pair, see:
// https://mathml-refresh.github.io/mathml-core/#operator-dictionary
// The returned value is never MathMLOperatorDictionaryCategory::kUndefined.
PLATFORM_EXPORT MathMLOperatorDictionaryCategory
FindCategory(const String& content, MathMLOperatorDictionaryForm);
......
......@@ -1249,6 +1249,7 @@ crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-axis-height-
crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-form-dynamic.html [ Failure ]
crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-form-minus-plus.html [ Failure ]
crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-form.html [ Failure ]
crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-not-in-dictionary-lspace-rspace.html [ Failure ]
crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-paint-lspace-rspace.html [ Failure ]
crbug.com/6606 external/wpt/mathml/presentation-markup/spaces/space-like-001.html [ Failure ]
crbug.com/6606 external/wpt/mathml/presentation-markup/spaces/space-like-002.html [ Failure ]
......
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8"/>
<title>lspace/rspace default value for unknown operators (reference)</title>
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
math {
font: 100px/1 Ahem;
}
mo {
color: blue;
}
</style>
</head>
<body>
<p>This test passes if on each row, the space around the blue rectangle
is 0.2777777777777778em.</p>
<p>
<math>
<mrow>
<mn>p</mn>
<mo lspace="0.2777777777777778em" rspace="0.2777777777777778em">X</mo> <!-- Single character -->
<mn>p</mn>
</mrow>
</math>
</p>
<p>
<math>
<mrow>
<mn>p</mn>
<mo lspace="0.2777777777777778em" rspace="0.2777777777777778em">XX</mo> <!-- Multiple characters -->
<mn>p</mn>
</mrow>
</math>
</p>
<p>
<math>
<mrow>
<mn>p</mn>
<mo form="infix" lspace="0.2777777777777778em" rspace="0.2777777777777778em">X</mo> <!-- Explicit form -->
<mn>p</mn>
</mrow>
</math>
</p>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8"/>
<title>lspace/rspace default value for unknown operators</title>
<link rel="help" href="https://mathml-refresh.github.io/mathml-core/#operator-fence-separator-or-accent-mo">
<meta name="assert" content="Verifies that the default lspace/rspace are 0.2777777777777778em for entries that are not in the operator dictionary.">
<link rel="match" href="mo-not-in-dictionary-lspace-rspace-ref.html">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
math {
font: 100px/1 Ahem;
}
mo {
color: blue;
}
</style>
</head>
<body>
<p>This test passes if on each row, the space around the blue rectangle
is 0.2777777777777778em.</p>
<p>
<math>
<mrow>
<mn>p</mn>
<mo>X</mo> <!-- Single character -->
<mn>p</mn>
</mrow>
</math>
</p>
<p>
<math>
<mrow>
<mn>p</mn>
<mo>XX</mo> <!-- Multiple characters -->
<mn>p</mn>
</mrow>
</math>
</p>
<p>
<math>
<mrow>
<mn>p</mn>
<mo form="infix">X</mo> <!-- Explicit form -->
<mn>p</mn>
</mrow>
</math>
</p>
<script src="/mathml/support/feature-detection.js"></script>
<script>MathMLFeatureDetection.ensure_for_match_reftest("has_operator_spacing");</script>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8"/>
<title>movablelimits default value for unknown operators (reference)</title>
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
math {
font: 50px/1 Ahem;
}
mo {
color: blue;
}
</style>
</head>
<body>
<p>This test passes if the black scripts are attached above and below
the corresponding blue base.</p>
<p>
<math>
<munderover>
<mo movablelimits="false">X</mo> <!-- Single character -->
<mn>X</mn>
<mn>X</mn>
</munderover>
</math>
</p>
<p>
<math>
<munderover>
<mo movablelimits="false">XX</mo> <!-- Multiple characters -->
<mn>X</mn>
<mn>X</mn>
</munderover>
</math>
</p>
<p>
<math>
<munderover>
<mo movablelimits="false" form="infix">X</mo> <!-- Explicit form -->
<mn>X</mn>
<mn>X</mn>
</munderover>
</math>
</p>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8"/>
<title>movablelimits default value for unknown operators</title>
<link rel="help" href="https://mathml-refresh.github.io/mathml-core/#operator-fence-separator-or-accent-mo">
<meta name="assert" content="Verifies that the default movablelimits is false for entries that are not in the operator dictionary.">
<link rel="match" href="mo-not-in-dictionary-movablelimits-ref.html">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
math {
font: 50px/1 Ahem;
}
mo {
color: blue;
}
</style>
</head>
<body>
<p>This test passes if the black scripts are attached above and below
the corresponding blue base.</p>
<p>
<math>
<munderover>
<mo>X</mo> <!-- Single character -->
<mn>X</mn>
<mn>X</mn>
</munderover>
</math>
</p>
<p>
<math>
<munderover>
<mo>XX</mo> <!-- Multiple characters -->
<mn>X</mn>
<mn>X</mn>
</munderover>
</math>
</p>
<p>
<math>
<munderover>
<mo form="infix">X</mo> <!-- Explicit form -->
<mn>X</mn>
<mn>X</mn>
</munderover>
</math>
</p>
<script src="/mathml/support/feature-detection.js"></script>
<script>MathMLFeatureDetection.ensure_for_match_reftest("has_movablelimits");</script>
</body>
</html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment