Skip to content

Commit

Permalink
[mathml] Support multi-keyword syntax for display
Browse files Browse the repository at this point in the history
Support multi-keyword syntax for display but only for MathML.
Specifically "inline math", "math inline" will be parsed and
serialized as "math", and "block math", "math block" will
be serialized as "block math".
Existing support for "inline-math" is removed since instead the
multi-keyword syntax was chosen [1].

[1] w3c/csswg-drafts#5385 (comment)

Bug: 6606, 1127222, 995106

Change-Id: Ibc9193dc8a2b09f4463f06c3de31e500ef45b1d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421598
Commit-Queue: Rob Buis <rbuis@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#811964}
GitOrigin-RevId: 6fbfb08fb4af24d97db9956fe8b8639f83866aae
  • Loading branch information
rwlbuis authored and copybara-github committed Sep 30, 2020
1 parent e06f88b commit 7074013
Show file tree
Hide file tree
Showing 20 changed files with 166 additions and 81 deletions.
1 change: 0 additions & 1 deletion blink/renderer/core/css/css.dict
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@
"grid"
"inline-grid"
"math"
"inline-math"
"contents"
"-webkit-flex"
"-webkit-inline-flex"
Expand Down
4 changes: 0 additions & 4 deletions blink/renderer/core/css/css_value_id_mappings.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,6 @@ inline EDisplay CssValueIDToPlatformEnum(CSSValueID v) {
return EDisplay::kInlineFlex;
if (v == CSSValueID::kMath)
return EDisplay::kMath;
if (v == CSSValueID::kInlineMath)
return EDisplay::kInlineMath;

NOTREACHED();
return EDisplay::kInline;
Expand Down Expand Up @@ -359,8 +357,6 @@ inline CSSValueID PlatformEnumToCSSValueID(EDisplay v) {
return CSSValueID::kContents;
if (v == EDisplay::kMath)
return CSSValueID::kMath;
if (v == EDisplay::kInlineMath)
return CSSValueID::kInlineMath;

NOTREACHED();
return CSSValueID::kInline;
Expand Down
29 changes: 15 additions & 14 deletions blink/renderer/core/css/css_value_keywords.json5
Original file line number Diff line number Diff line change
Expand Up @@ -489,16 +489,19 @@
"katakana-iroha",
//none
//
// display
// The order of this enum must match the order found in CSSParserFastPaths::IsValidKeywordPropertyAndValue().
//
// The order must match the order found in IsDisplayOutside().
"inline",
"block",
// The order must match the order found in IsDisplayInside().
"flow-root",
"list-item",
"inline-block",
"table",
"inline-table",
"flex",
"grid",
"math",
// The order must match the order found in IsDisplayBox().
"contents",
//none
// The order must match the order found in IsDisplayInternal().
"table-row-group",
"table-header-group",
"table-footer-group",
Expand All @@ -507,20 +510,18 @@
"table-column",
"table-cell",
"table-caption",
"-webkit-box",
"-webkit-inline-box",
"flex",
// The order must match the order found in IsDisplayLegacy().
"inline-block",
"inline-table",
"inline-flex",
"grid",
"inline-grid",
"math",
"inline-math",
"contents",
//none
"-webkit-box",
"-webkit-inline-box",
"-webkit-flex",
"-webkit-inline-flex",
"layout",
"inline-layout",
"list-item",
//
// cursor
// The order of this enum must match the order found in CSSPropertyParser::ConsumeCursor().
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/css/mathml.css
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@

/* Default display */
* {
display: math;
display: block math;
font-size: math;
}

/* By default, we only display the MathML formulas without any formatting other than the one specified by the display attribute. */
math {
display: inline-math;
display: inline math;
math-shift: normal;
math-style: compact;
font-size: inherited;
math-depth: 0;
}

math[display="block"] {
display: math;
display: block math;
text-align: center;
}

Expand Down
13 changes: 0 additions & 13 deletions blink/renderer/core/css/parser/css_parser_fast_paths.cc
Original file line number Diff line number Diff line change
Expand Up @@ -622,17 +622,6 @@ bool CSSParserFastPaths::IsValidKeywordPropertyAndValue(
value_id == CSSValueID::kOptimizequality;
case CSSPropertyID::kDirection:
return value_id == CSSValueID::kLtr || value_id == CSSValueID::kRtl;
case CSSPropertyID::kDisplay:
return (value_id >= CSSValueID::kInline &&
value_id <= CSSValueID::kInlineFlex) ||
value_id == CSSValueID::kWebkitFlex ||
value_id == CSSValueID::kWebkitInlineFlex ||
value_id == CSSValueID::kNone || value_id == CSSValueID::kGrid ||
value_id == CSSValueID::kInlineGrid ||
value_id == CSSValueID::kContents ||
(RuntimeEnabledFeatures::MathMLCoreEnabled() &&
(value_id == CSSValueID::kMath ||
value_id == CSSValueID::kInlineMath));
case CSSPropertyID::kDominantBaseline:
return value_id == CSSValueID::kAuto ||
value_id == CSSValueID::kAlphabetic ||
Expand Down Expand Up @@ -1034,7 +1023,6 @@ bool CSSParserFastPaths::IsKeywordPropertyID(CSSPropertyID property_id) {
case CSSPropertyID::kColorInterpolationFilters:
case CSSPropertyID::kColorRendering:
case CSSPropertyID::kDirection:
case CSSPropertyID::kDisplay:
case CSSPropertyID::kDominantBaseline:
case CSSPropertyID::kEmptyCells:
case CSSPropertyID::kFillRule:
Expand Down Expand Up @@ -1132,7 +1120,6 @@ bool CSSParserFastPaths::IsKeywordPropertyID(CSSPropertyID property_id) {

bool CSSParserFastPaths::IsPartialKeywordPropertyID(CSSPropertyID property_id) {
switch (property_id) {
case CSSPropertyID::kDisplay:
case CSSPropertyID::kListStyleType:
return true;
default:
Expand Down
8 changes: 4 additions & 4 deletions blink/renderer/core/css/parser/css_parser_fast_paths_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ TEST(CSSParserFastPathsTest, ParseRevert) {

// Revert enabled, IsKeywordPropertyID=true
{
DCHECK(CSSParserFastPaths::IsKeywordPropertyID(CSSPropertyID::kDisplay));
DCHECK(CSSParserFastPaths::IsKeywordPropertyID(CSSPropertyID::kDirection));
ScopedCSSRevertForTest scoped_revert(true);
CSSValue* value = CSSParserFastPaths::MaybeParseValue(
CSSPropertyID::kDisplay, "revert", kHTMLStandardMode);
CSSPropertyID::kDirection, "revert", kHTMLStandardMode);
ASSERT_TRUE(value);
EXPECT_TRUE(value->IsRevertValue());
}
Expand All @@ -86,10 +86,10 @@ TEST(CSSParserFastPathsTest, ParseRevert) {

// Revert disabled, IsKeywordPropertyID=true
{
DCHECK(CSSParserFastPaths::IsKeywordPropertyID(CSSPropertyID::kDisplay));
DCHECK(CSSParserFastPaths::IsKeywordPropertyID(CSSPropertyID::kDirection));
ScopedCSSRevertForTest scoped_revert(false);
CSSValue* value = CSSParserFastPaths::MaybeParseValue(
CSSPropertyID::kDisplay, "revert", kHTMLStandardMode);
CSSPropertyID::kDirection, "revert", kHTMLStandardMode);
EXPECT_FALSE(value);
}
}
Expand Down
93 changes: 91 additions & 2 deletions blink/renderer/core/css/properties/longhands/longhands_custom.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2294,11 +2294,72 @@ void Direction::ApplyValue(StyleResolverState& state,
To<CSSIdentifierValue>(value).ConvertTo<TextDirection>());
}

namespace {

static bool IsDisplayOutside(CSSValueID id) {
return id >= CSSValueID::kInline && id <= CSSValueID::kBlock;
}

static bool IsDisplayInside(CSSValueID id) {
if (id >= CSSValueID::kFlowRoot && id <= CSSValueID::kGrid)
return true;
if (id == CSSValueID::kMath)
return RuntimeEnabledFeatures::MathMLCoreEnabled();
return false;
}

static bool IsDisplayBox(CSSValueID id) {
return css_parsing_utils::IdentMatches<CSSValueID::kNone,
CSSValueID::kContents>(id);
}

static bool IsDisplayInternal(CSSValueID id) {
return id >= CSSValueID::kTableRowGroup && id <= CSSValueID::kTableCaption;
}

static bool IsDisplayLegacy(CSSValueID id) {
return id >= CSSValueID::kInlineBlock && id <= CSSValueID::kWebkitInlineFlex;
}

} // namespace

const CSSValue* Display::ParseSingleValue(CSSParserTokenRange& range,
const CSSParserContext& context,
const CSSParserLocalContext&) const {
// NOTE: All the keyword values for the display property are handled by the
// CSSParserFastPaths.
CSSValueID id = range.Peek().Id();
CSSIdentifierValue* display_outside = nullptr;
CSSIdentifierValue* display_inside = nullptr;
if (IsDisplayOutside(id)) {
display_outside = css_parsing_utils::ConsumeIdent(range);
if (range.AtEnd())
return display_outside;
id = range.Peek().Id();
if (!IsDisplayInside(id))
return nullptr;
display_inside = css_parsing_utils::ConsumeIdent(range);
} else if (IsDisplayInside(id)) {
display_inside = css_parsing_utils::ConsumeIdent(range);
if (range.AtEnd())
return display_inside;
id = range.Peek().Id();
if (!IsDisplayOutside(id))
return nullptr;
display_outside = css_parsing_utils::ConsumeIdent(range);
}
if (display_outside && display_inside) {
// TODO(crbug.com/995106): should apply to more than just math.
if (display_inside->GetValueID() == CSSValueID::kMath) {
CSSValueList* parsed_values = CSSValueList::CreateSpaceSeparated();
parsed_values->Append(*display_outside);
parsed_values->Append(*display_inside);
return parsed_values;
}
return nullptr;
}
if (id == CSSValueID::kListItem || IsDisplayBox(id) ||
IsDisplayInternal(id) || IsDisplayLegacy(id))
return css_parsing_utils::ConsumeIdent(range);

if (!RuntimeEnabledFeatures::CSSLayoutAPIEnabled())
return nullptr;

Expand Down Expand Up @@ -2336,6 +2397,16 @@ const CSSValue* Display::CSSValueFromComputedStyleInternal(
style.IsDisplayInlineType());
}

if (style.Display() == EDisplay::kBlockMath) {
CSSValueList* values = CSSValueList::CreateSpaceSeparated();
if (style.Display() == EDisplay::kBlockMath)
values->Append(*CSSIdentifierValue::Create(CSSValueID::kBlock));
else
values->Append(*CSSIdentifierValue::Create(CSSValueID::kInline));
values->Append(*CSSIdentifierValue::Create(CSSValueID::kMath));
return values;
}

return CSSIdentifierValue::Create(style.Display());
}

Expand All @@ -2360,6 +2431,24 @@ void Display::ApplyValue(StyleResolverState& state,
return;
}

if (value.IsValueList()) {
state.Style()->SetDisplayLayoutCustomName(
ComputedStyleInitialValues::InitialDisplayLayoutCustomName());
const CSSValueList& display_pair = To<CSSValueList>(value);
DCHECK_EQ(display_pair.length(), 2u);
DCHECK(display_pair.Item(0).IsIdentifierValue());
DCHECK(display_pair.Item(1).IsIdentifierValue());
const auto& outside = To<CSSIdentifierValue>(display_pair.Item(0));
const auto& inside = To<CSSIdentifierValue>(display_pair.Item(1));
// TODO(crbug.com/995106): should apply to more than just math.
DCHECK(inside.GetValueID() == CSSValueID::kMath);
if (outside.GetValueID() == CSSValueID::kBlock)
state.Style()->SetDisplay(EDisplay::kBlockMath);
else
state.Style()->SetDisplay(EDisplay::kMath);
return;
}

const auto& layout_function_value =
To<cssvalue::CSSLayoutFunctionValue>(value);

Expand Down
14 changes: 7 additions & 7 deletions blink/renderer/core/css/resolver/style_adjuster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ static EDisplay EquivalentBlockDisplay(EDisplay display) {
case EDisplay::kWebkitBox:
case EDisplay::kFlex:
case EDisplay::kGrid:
case EDisplay::kMath:
case EDisplay::kBlockMath:
case EDisplay::kListItem:
case EDisplay::kFlowRoot:
case EDisplay::kLayoutCustom:
Expand All @@ -141,8 +141,8 @@ static EDisplay EquivalentBlockDisplay(EDisplay display) {
return EDisplay::kFlex;
case EDisplay::kInlineGrid:
return EDisplay::kGrid;
case EDisplay::kInlineMath:
return EDisplay::kMath;
case EDisplay::kMath:
return EDisplay::kBlockMath;
case EDisplay::kInlineLayoutCustom:
return EDisplay::kLayoutCustom;

Expand Down Expand Up @@ -666,10 +666,10 @@ void StyleAdjuster::AdjustComputedStyle(StyleResolverState& state,
// math display values on non-MathML elements compute to flow display
// values.
if ((!element || !element->IsMathMLElement()) &&
(style.Display() == EDisplay::kMath ||
style.Display() == EDisplay::kInlineMath)) {
style.SetDisplay(style.Display() == EDisplay::kMath ? EDisplay::kBlock
: EDisplay::kInline);
style.IsDisplayMathBox(style.Display())) {
style.SetDisplay(style.Display() == EDisplay::kBlockMath
? EDisplay::kBlock
: EDisplay::kInline);
}

// We don't adjust the first letter style earlier because we may change the
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/layout/layout_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ LayoutObject* LayoutObject::CreateObject(Element* element,
UseCounter::Count(element->GetDocument(), WebFeature::kCSSGridLayout);
return LayoutObjectFactory::CreateGrid(*element, style, legacy);
case EDisplay::kMath:
case EDisplay::kInlineMath:
case EDisplay::kBlockMath:
return LayoutObjectFactory::CreateMath(*element, style, legacy);
case EDisplay::kLayoutCustom:
case EDisplay::kInlineLayoutCustom:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ void LayoutNGMathMLBlockWithAnonymousMrow::AddChild(
if (!anonymous_mrow) {
scoped_refptr<ComputedStyle> new_style =
ComputedStyle::CreateAnonymousStyleWithDisplay(StyleRef(),
EDisplay::kMath);
EDisplay::kBlockMath);

UpdateAnonymousChildStyle(nullptr, *new_style);
anonymous_mrow = new LayoutNGMathMLBlock(nullptr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ namespace blink {
namespace {

inline LayoutUnit InlineOffsetForDisplayMathCentering(
bool is_display_math,
bool is_display_block_math,
LayoutUnit available_inline_size,
LayoutUnit max_row_inline_size) {
if (is_display_math)
if (is_display_block_math)
return (available_inline_size - max_row_inline_size) / 2;
return LayoutUnit();
}
Expand Down Expand Up @@ -116,8 +116,8 @@ void NGMathRowLayoutAlgorithm::LayoutRowItems(
scoped_refptr<const NGLayoutResult> NGMathRowLayoutAlgorithm::Layout() {
DCHECK(!BreakToken());

bool is_display_math =
Node().IsMathRoot() && Style().Display() == EDisplay::kMath;
bool is_display_block_math =
Node().IsMathRoot() && (Style().Display() == EDisplay::kBlockMath);

LogicalSize max_row_size;
LayoutUnit max_row_block_baseline;
Expand All @@ -130,7 +130,7 @@ scoped_refptr<const NGLayoutResult> NGMathRowLayoutAlgorithm::Layout() {
// Add children taking into account centering, baseline and
// border/scrollbar/padding.
LayoutUnit center_offset = InlineOffsetForDisplayMathCentering(
is_display_math, container_builder_.InlineSize(),
is_display_block_math, container_builder_.InlineSize(),
max_row_size.inline_size);

LogicalOffset adjust_offset = BorderScrollbarPadding().StartOffset();
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/mathml/mathml_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ void MathMLElement::CollectStyleForPresentationAttribute(
HasTagName(mathml_names::kMathTag)) {
if (EqualIgnoringASCIICase(value, "inline")) {
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kDisplay,
CSSValueID::kInlineMath);
CSSValueID::kMath);
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kMathStyle,
CSSValueID::kCompact);
} else if (EqualIgnoringASCIICase(value, "block")) {
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kDisplay,
CSSValueID::kMath);
"block math");
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kMathStyle,
CSSValueID::kNormal);
}
Expand Down
Loading

0 comments on commit 7074013

Please sign in to comment.