Skip to content

Commit

Permalink
[balance-text] Remove CSSWhiteSpaceShorthand runtime flag
Browse files Browse the repository at this point in the history
Shipped in M114.

crrev.com/c/4751002 removed the related flag `CSSTextWrap`,
but this flag is a bit more complicated, by using the
`alternative_of` keyword, and also impacts editing and
presentation attributes.

Bug: 1251079, 1417543
Change-Id: I59460711d56d1e32185294794c10f18378884ba7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4751154
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1182125}
  • Loading branch information
kojiishi authored and Chromium LUCI CQ committed Aug 10, 2023
1 parent 46c4eb1 commit a1263ce
Show file tree
Hide file tree
Showing 18 changed files with 49 additions and 188 deletions.
Expand Up @@ -2222,14 +2222,6 @@ void CSSAnimations::CalculateTransitionUpdateForPropertyHandle(
return;
}

if (RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled() &&
property.GetCSSProperty().PropertyID() == CSSPropertyID::kWhiteSpace) {
// When CSSWhiteSpaceShorthand is enabled white-space is a shorthand, so we
// shouldn't transition it. Otherwise, we will hit the DCHECK in
// WhiteSpace::CSSValueFromComputedStyleInternal.
return;
}

const RunningTransition* interrupted_transition = nullptr;
if (state.active_transitions) {
TransitionMap::const_iterator active_transition_iter =
Expand Down
24 changes: 0 additions & 24 deletions third_party/blink/renderer/core/css/css_properties.json5
Expand Up @@ -6098,33 +6098,10 @@
},
{
name: "white-space",
property_methods: ["CSSValueFromComputedStyleInternal"],
independent: false, // Actually true, but setting it to false saves a precious bit in ComputedStyleBase.
inherited: true,
// This property is "keyword" but use "primitive" to use `EWhiteSpace` defined manually.
field_template: "primitive",
// The values are stored in `white-space-collapse` and `text-wrap` by
// `computed_style_custom_functions` even when `CSSWhiteSpaceShorthand`
// is off, but give 1 bit to it because `field_size` can't be zero.
field_size: 1,
include_paths: ["third_party/blink/renderer/core/css/white_space.h"],
keywords: [
"normal", "pre", "pre-wrap", "pre-line", "nowrap", "break-spaces"
],
computed_style_custom_functions: ["getter", "setter", "resetter"],
typedom_types: ["Keyword"],
default_value: "EWhiteSpace::kNormal",
valid_for_cue: true,
valid_for_marker: true,
},
{
name: "-alternative-white-space",
alternative_of: "white-space",
longhands: [
"white-space-collapse", "text-wrap"
],
property_methods: ["ParseShorthand", "CSSValueFromComputedStyleInternal"],
runtime_flag: "CSSWhiteSpaceShorthand",
},
{
name: "white-space-collapse",
Expand All @@ -6139,7 +6116,6 @@
typedom_types: ["Keyword"],
valid_for_cue: true,
valid_for_marker: true,
runtime_flag: "CSSWhiteSpaceShorthand",
},
{
name: "text-wrap",
Expand Down
4 changes: 1 addition & 3 deletions third_party/blink/renderer/core/css/css_property_equality.cc
Expand Up @@ -838,8 +838,6 @@ bool CSSPropertyEquality::PropertiesEqual(const PropertyHandle& property,
return a.GetTransformOrigin().Y() == b.GetTransformOrigin().Y();
case CSSPropertyID::kWebkitTransformOriginZ:
return a.GetTransformOrigin().Z() == b.GetTransformOrigin().Z();
case CSSPropertyID::kWhiteSpace:
return a.WhiteSpace() == b.WhiteSpace();
case CSSPropertyID::kWhiteSpaceCollapse:
return a.GetWhiteSpaceCollapse() == b.GetWhiteSpaceCollapse();
case CSSPropertyID::kWidows:
Expand Down Expand Up @@ -1217,7 +1215,7 @@ bool CSSPropertyEquality::PropertiesEqual(const PropertyHandle& property,
case CSSPropertyID::kWebkitMaskPosition:
case CSSPropertyID::kWebkitMaskRepeat:
case CSSPropertyID::kWebkitTextStroke:
case CSSPropertyID::kAlternativeWhiteSpace:
case CSSPropertyID::kWhiteSpace:
NOTREACHED() << property.GetCSSPropertyName().ToAtomicString().Ascii();
return true;

Expand Down
9 changes: 2 additions & 7 deletions third_party/blink/renderer/core/css/css_property_value_set.cc
Expand Up @@ -448,8 +448,7 @@ void MutableCSSPropertyValueSet::SetProperty(CSSPropertyID property_id,
const CSSValue& value,
bool important) {
DCHECK_NE(property_id, CSSPropertyID::kVariable);
DCHECK(!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled() ||
property_id != CSSPropertyID::kWhiteSpace);
DCHECK_NE(property_id, CSSPropertyID::kWhiteSpace);
StylePropertyShorthand shorthand = shorthandForProperty(property_id);
if (!shorthand.length()) {
SetLonghandProperty(
Expand All @@ -460,7 +459,7 @@ void MutableCSSPropertyValueSet::SetProperty(CSSPropertyID property_id,
RemovePropertiesInSet(shorthand.properties(), shorthand.length());

// The simple shorthand expansion below doesn't work for `white-space`.
DCHECK_NE(property_id, CSSPropertyID::kAlternativeWhiteSpace);
DCHECK_NE(property_id, CSSPropertyID::kWhiteSpace);
for (unsigned i = 0; i < shorthand.length(); ++i) {
CSSPropertyName longhand_name(shorthand.properties()[i]->PropertyID());
property_vector_.push_back(
Expand Down Expand Up @@ -498,8 +497,6 @@ MutableCSSPropertyValueSet::SetLonghandProperty(CSSPropertyValue property) {
const CSSPropertyID id = property.Id();
DCHECK_EQ(shorthandForProperty(id).length(), 0u)
<< CSSProperty::Get(id).GetPropertyNameString() << " is a shorthand";
DCHECK(!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled() ||
id != CSSPropertyID::kWhiteSpace);
CSSPropertyValue* to_replace;
if (id == CSSPropertyID::kVariable) {
to_replace = const_cast<CSSPropertyValue*>(
Expand All @@ -525,8 +522,6 @@ void MutableCSSPropertyValueSet::SetLonghandProperty(CSSPropertyID property_id,
DCHECK_EQ(shorthandForProperty(property_id).length(), 0u)
<< CSSProperty::Get(property_id).GetPropertyNameString()
<< " is a shorthand";
DCHECK(!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled() ||
property_id != CSSPropertyID::kWhiteSpace);
CSSPropertyValue* to_replace = FindInsertionPointForID(property_id);
if (to_replace) {
*to_replace = CSSPropertyValue(CSSPropertyName(property_id), value);
Expand Down
13 changes: 0 additions & 13 deletions third_party/blink/renderer/core/css/css_value_id_mappings.h
Expand Up @@ -403,13 +403,6 @@ inline CSSValueID PlatformEnumToCSSValueID(EWhiteSpace v) {
case EWhiteSpace::kBreakSpaces:
return CSSValueID::kBreakSpaces;
}
if (ToTextWrap(v) == TextWrap::kBalance &&
!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
// If `text-wrap: balance` but the shorthandifying `white-space` is off,
// pretend as if `text-wrap: wrap`.
return PlatformEnumToCSSValueID(
ToWhiteSpace(ToWhiteSpaceCollapse(v), TextWrap::kWrap));
}
NOTREACHED();
return CSSValueID::kNone;
}
Expand Down Expand Up @@ -453,7 +446,6 @@ inline TextWrap CssValueIDToPlatformEnum(CSSValueID v) {
case CSSValueID::kWrap:
return TextWrap::kWrap;
case CSSValueID::kNowrap:
DCHECK(RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled());
return TextWrap::kNoWrap;
case CSSValueID::kBalance:
return TextWrap::kBalance;
Expand All @@ -472,11 +464,6 @@ inline CSSValueID PlatformEnumToCSSValueID(TextWrap v) {
case TextWrap::kWrap:
return CSSValueID::kWrap;
case TextWrap::kNoWrap:
if (!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
// Note this is not right, but a compromise until `white-space` becomes
// a shorthand. Simulate the behavior when it's off.
return CSSValueID::kWrap;
}
return CSSValueID::kNowrap;
case TextWrap::kBalance:
return CSSValueID::kBalance;
Expand Down
Expand Up @@ -1532,10 +1532,6 @@ bool CSSParserFastPaths::IsValidKeywordPropertyAndValue(
return value_id == CSSValueID::kDisc || value_id == CSSValueID::kCircle ||
value_id == CSSValueID::kSquare || value_id == CSSValueID::kNone;
case CSSPropertyID::kTextWrap:
if (!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
return value_id == CSSValueID::kWrap ||
value_id == CSSValueID::kBalance;
}
if (!RuntimeEnabledFeatures::CSSTextWrapPrettyEnabled()) {
return value_id == CSSValueID::kWrap ||
value_id == CSSValueID::kNowrap ||
Expand Down Expand Up @@ -1582,15 +1578,7 @@ bool CSSParserFastPaths::IsValidKeywordPropertyAndValue(
value_id == CSSValueID::kLrTb || value_id == CSSValueID::kRlTb ||
value_id == CSSValueID::kTbRl || value_id == CSSValueID::kLr ||
value_id == CSSValueID::kRl || value_id == CSSValueID::kTb;
case CSSPropertyID::kWhiteSpace:
DCHECK(!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled());
return value_id == CSSValueID::kNormal || value_id == CSSValueID::kPre ||
value_id == CSSValueID::kPreWrap ||
value_id == CSSValueID::kPreLine ||
value_id == CSSValueID::kNowrap ||
value_id == CSSValueID::kBreakSpaces;
case CSSPropertyID::kWhiteSpaceCollapse:
DCHECK(RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled());
return value_id == CSSValueID::kCollapse ||
value_id == CSSValueID::kPreserve ||
value_id == CSSValueID::kPreserveBreaks ||
Expand Down Expand Up @@ -1745,7 +1733,6 @@ CSSBitset CSSParserFastPaths::handled_by_keyword_fast_paths_properties_{{
CSSPropertyID::kWebkitUserModify,
CSSPropertyID::kUserSelect,
CSSPropertyID::kWebkitWritingMode,
CSSPropertyID::kWhiteSpace, // TODO(crbug.com/1417543): Remove when done.
CSSPropertyID::kWhiteSpaceCollapse,
CSSPropertyID::kWordBreak,
CSSPropertyID::kWritingMode,
Expand Down
Expand Up @@ -10130,14 +10130,6 @@ const CSSValue* WebkitWritingMode::CSSValueFromComputedStyleInternal(
return CSSIdentifierValue::Create(style.GetWritingMode());
}

const CSSValue* WhiteSpace::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
const LayoutObject*,
bool allow_visited_style) const {
DCHECK(!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled());
return CSSIdentifierValue::Create(style.WhiteSpace());
}

// Longhands for `white-space`: `white-space-collapse` and `text-wrap`.
const CSSValue* WhiteSpaceCollapse::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
Expand Down
Expand Up @@ -3862,7 +3862,7 @@ const CSSValue* Toggle::CSSValueFromComputedStyleInternal(
return toggle_root;
}

bool AlternativeWhiteSpace::ParseShorthand(
bool WhiteSpace::ParseShorthand(
bool important,
CSSParserTokenRange& range,
const CSSParserContext& context,
Expand Down Expand Up @@ -3901,10 +3901,10 @@ bool AlternativeWhiteSpace::ParseShorthand(

// Consume multi-value syntax if the first identifier is not pre-defined.
return css_parsing_utils::ConsumeShorthandGreedilyViaLonghands(
alternativeWhiteSpaceShorthand(), important, context, range, properties);
whiteSpaceShorthand(), important, context, range, properties);
}

const CSSValue* AlternativeWhiteSpace::CSSValueFromComputedStyleInternal(
const CSSValue* WhiteSpace::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
const LayoutObject* layout_object,
bool allow_visited_style) const {
Expand Down
15 changes: 0 additions & 15 deletions third_party/blink/renderer/core/css/resolver/style_cascade.cc
Expand Up @@ -539,21 +539,6 @@ void StyleCascade::ApplyWideOverlapping(CascadeResolver& resolver) {
maybe_skip(GetCSSPropertyBaselineSource(), *priority);
}
}

if (!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
// TODO(crbug.com/1417543): `white-space` will become a shorthand in the
// future - in order to mitigate the forward compat risk, skip the
// `text-wrap` longhand.
const CSSProperty& white_space = GetCSSPropertyWhiteSpace();
DCHECK(white_space.IsLonghand());
if (!resolver.filter_.Rejects(white_space)) {
if (const CascadePriority* priority =
map_.Find(white_space.GetCSSPropertyName())) {
LookupAndApply(white_space, resolver);
maybe_skip(GetCSSPropertyTextWrap(), *priority);
}
}
}
}

// Go through all properties that were found during the analyze phase
Expand Down
Expand Up @@ -400,7 +400,7 @@ static bool AllowInitialInShorthand(CSSPropertyID property_id) {
case CSSPropertyID::kTextEmphasis:
case CSSPropertyID::kWebkitMask:
case CSSPropertyID::kWebkitTextStroke:
case CSSPropertyID::kAlternativeWhiteSpace:
case CSSPropertyID::kWhiteSpace:
return true;
default:
return false;
Expand Down Expand Up @@ -682,7 +682,7 @@ String StylePropertySerializer::SerializeShorthand(
}
case CSSPropertyID::kViewTimeline:
return ViewTimelineValue();
case CSSPropertyID::kAlternativeWhiteSpace:
case CSSPropertyID::kWhiteSpace:
return WhiteSpaceValue();
case CSSPropertyID::kGridColumnGap:
case CSSPropertyID::kGridGap:
Expand Down Expand Up @@ -2285,8 +2285,6 @@ bool StylePropertySerializer::IsValidToggleShorthand(
}

String StylePropertySerializer::WhiteSpaceValue() const {
DCHECK(RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled());

const CSSValue* collapse_value =
property_set_.GetPropertyCSSValue(GetCSSPropertyWhiteSpaceCollapse());
const CSSValue* wrap_value =
Expand Down
3 changes: 1 addition & 2 deletions third_party/blink/renderer/core/dom/element.cc
Expand Up @@ -8002,8 +8002,7 @@ void Element::AddPropertyToPresentationAttributeStyle(
CSSPropertyID property_id,
CSSValueID identifier) {
DCHECK(IsStyledElement());
DCHECK(!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled() ||
property_id != CSSPropertyID::kWhiteSpace);
DCHECK_NE(property_id, CSSPropertyID::kWhiteSpace);
style->SetLonghandProperty(property_id,
*CSSIdentifierValue::Create(identifier));
}
Expand Down
21 changes: 7 additions & 14 deletions third_party/blink/renderer/core/editing/editing_style.cc
Expand Up @@ -100,7 +100,10 @@ static const CSSPropertyID kStaticEditingProperties[] = {
CSSPropertyID::kWebkitTextFillColor,
CSSPropertyID::kWebkitTextStrokeColor,
CSSPropertyID::kWebkitTextStrokeWidth,
CSSPropertyID::kCaretColor};
CSSPropertyID::kCaretColor,
CSSPropertyID::kTextWrap,
CSSPropertyID::kWhiteSpaceCollapse,
};

enum EditingPropertiesType {
kOnlyInheritableEditingProperties,
Expand All @@ -115,14 +118,6 @@ static const Vector<const CSSProperty*>& AllEditingProperties(
CSSProperty::FilterWebExposedCSSPropertiesIntoVector(
execution_context, kStaticEditingProperties,
std::size(kStaticEditingProperties), properties);
// TODO(crbug.com/1417543): Move to `kStaticEditingProperties` when removing
// the runtime switch.
if (RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
properties.push_back(&GetCSSPropertyWhiteSpaceCollapse());
properties.push_back(&GetCSSPropertyTextWrap());
} else {
properties.push_back(&GetCSSPropertyWhiteSpace());
}
}
return properties;
}
Expand Down Expand Up @@ -1054,12 +1049,10 @@ bool EditingStyle::ConflictsWithInlineStyleOfElement(
// e-mail, etc., `white-space` is more interoperable when
// `white-space-collapse` is not broadly supported. See crbug.com/1417543
// and `editing/pasteboard/pasting-tabs.html`.
DCHECK_NE(property_id, CSSPropertyID::kAlternativeWhiteSpace);
DCHECK_NE(property_id, CSSPropertyID::kWhiteSpace);
const bool is_whitespace_property =
RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()
? property_id == CSSPropertyID::kWhiteSpaceCollapse ||
property_id == CSSPropertyID::kTextWrap
: property_id == CSSPropertyID::kWhiteSpace;
property_id == CSSPropertyID::kWhiteSpaceCollapse ||
property_id == CSSPropertyID::kTextWrap;
if (is_whitespace_property && IsTabHTMLSpanElement(element)) {
continue;
}
Expand Down
Expand Up @@ -187,29 +187,19 @@ void HTMLTextAreaElement::CollectStyleForPresentationAttribute(
MutableCSSPropertyValueSet* style) {
if (name == html_names::kWrapAttr) {
if (ShouldWrapText()) {
if (!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kWhiteSpace, CSSValueID::kPreWrap);
} else {
// Longhands of `white-space: pre-wrap`.
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kWhiteSpaceCollapse, CSSValueID::kPreserve);
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kTextWrap,
CSSValueID::kWrap);
}
// Longhands of `white-space: pre-wrap`.
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kWhiteSpaceCollapse, CSSValueID::kPreserve);
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kTextWrap,
CSSValueID::kWrap);
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kOverflowWrap, CSSValueID::kBreakWord);
} else {
if (!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kWhiteSpace, CSSValueID::kPre);
} else {
// Longhands of `white-space: pre`.
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kWhiteSpaceCollapse, CSSValueID::kPreserve);
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kTextWrap,
CSSValueID::kNowrap);
}
// Longhands of `white-space: pre`.
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kWhiteSpaceCollapse, CSSValueID::kPreserve);
AddPropertyToPresentationAttributeStyle(style, CSSPropertyID::kTextWrap,
CSSValueID::kNowrap);
AddPropertyToPresentationAttributeStyle(
style, CSSPropertyID::kOverflowWrap, CSSValueID::kNormal);
}
Expand Down
13 changes: 4 additions & 9 deletions third_party/blink/renderer/core/html/html_pre_element.cc
Expand Up @@ -44,15 +44,10 @@ void HTMLPreElement::CollectStyleForPresentationAttribute(
const AtomicString& value,
MutableCSSPropertyValueSet* style) {
if (name == html_names::kWrapAttr) {
if (!RuntimeEnabledFeatures::CSSWhiteSpaceShorthandEnabled()) {
style->SetLonghandProperty(CSSPropertyID::kWhiteSpace,
CSSValueID::kPreWrap);
} else {
// Longhands of `white-space: pre-wrap`.
style->SetLonghandProperty(CSSPropertyID::kWhiteSpaceCollapse,
CSSValueID::kPreserve);
style->SetLonghandProperty(CSSPropertyID::kTextWrap, CSSValueID::kWrap);
}
// Longhands of `white-space: pre-wrap`.
style->SetLonghandProperty(CSSPropertyID::kWhiteSpaceCollapse,
CSSValueID::kPreserve);
style->SetLonghandProperty(CSSPropertyID::kTextWrap, CSSValueID::kWrap);
} else {
HTMLElement::CollectStyleForPresentationAttribute(name, value, style);
}
Expand Down

0 comments on commit a1263ce

Please sign in to comment.