From 64e97bbea821e79baad75ddab58d5f3f4e1dfa1f Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Wed, 8 Apr 2015 13:39:47 -0700 Subject: [PATCH] remove CSS 'all' property. The human mind is simply not capable of beholding the sheer power of the CSS 'all' property. We shall remove it now and leave its echo for generations to contemplate. R=ojan@chromium.org Review URL: https://codereview.chromium.org/1068393002 --- .../core/css/CSSComputedStyleDeclaration.cpp | 3 -- engine/core/css/CSSProperties.in | 3 -- engine/core/css/CSSProperty.cpp | 14 ------ engine/core/css/CSSProperty.h | 1 - engine/core/css/parser/BisonCSSParser-in.cpp | 3 -- engine/core/css/parser/CSSPropertyParser.cpp | 2 +- engine/core/css/resolver/StyleResolver.cpp | 45 +------------------ engine/core/css/resolver/StyleResolver.h | 2 - 8 files changed, 2 insertions(+), 71 deletions(-) diff --git a/engine/core/css/CSSComputedStyleDeclaration.cpp b/engine/core/css/CSSComputedStyleDeclaration.cpp index df95d1be9e3..f1bc1a37dbc 100644 --- a/engine/core/css/CSSComputedStyleDeclaration.cpp +++ b/engine/core/css/CSSComputedStyleDeclaration.cpp @@ -2137,9 +2137,6 @@ PassRefPtr CSSComputedStyleDeclaration::getPropertyCSSValue(CSSPropert /* @viewport rule properties */ case CSSPropertyOrientation: break; - - case CSSPropertyAll: - return nullptr; } logUnimplementedPropertyID(propertyID); diff --git a/engine/core/css/CSSProperties.in b/engine/core/css/CSSProperties.in index bcb28fb3d34..0785c162fc2 100644 --- a/engine/core/css/CSSProperties.in +++ b/engine/core/css/CSSProperties.in @@ -499,9 +499,6 @@ z-index animatable, type_name=unsigned, custom_all // Properties that we ignore in the StyleBuilder. // FIXME: We should see if any of these should actually be unreachable -// OBSOLETE -all builder_skip - // OBSOLETE orientation builder_skip diff --git a/engine/core/css/CSSProperty.cpp b/engine/core/css/CSSProperty.cpp index ea33ca90b7f..2bc6f7a432a 100644 --- a/engine/core/css/CSSProperty.cpp +++ b/engine/core/css/CSSProperty.cpp @@ -172,18 +172,4 @@ CSSPropertyID CSSProperty::resolveDirectionAwareProperty(CSSPropertyID propertyI } } -bool CSSProperty::isAffectedByAllProperty(CSSPropertyID propertyID) -{ - if (propertyID == CSSPropertyAll) - return false; - - // all shorthand spec says: - // The all property is a shorthand that resets all CSS properties except - // direction and unicode-bidi. It only accepts the CSS-wide keywords. - // c.f. http://dev.w3.org/csswg/css-cascade/#all-shorthand - // So CSSPropertyUnicodeBidi and CSSPropertyDirection are not - // affected by all property. - return propertyID != CSSPropertyUnicodeBidi && propertyID != CSSPropertyDirection; -} - } // namespace blink diff --git a/engine/core/css/CSSProperty.h b/engine/core/css/CSSProperty.h index 050b34d7b05..9a39901fa46 100644 --- a/engine/core/css/CSSProperty.h +++ b/engine/core/css/CSSProperty.h @@ -75,7 +75,6 @@ public: void wrapValueInCommaSeparatedList(); static CSSPropertyID resolveDirectionAwareProperty(CSSPropertyID, TextDirection); - static bool isAffectedByAllProperty(CSSPropertyID); const StylePropertyMetadata& metadata() const { return m_metadata; } diff --git a/engine/core/css/parser/BisonCSSParser-in.cpp b/engine/core/css/parser/BisonCSSParser-in.cpp index ae8665480d5..9fdb154e609 100644 --- a/engine/core/css/parser/BisonCSSParser-in.cpp +++ b/engine/core/css/parser/BisonCSSParser-in.cpp @@ -317,8 +317,6 @@ bool isValidKeywordPropertyAndValue(CSSPropertyID propertyId, CSSValueID valueID return false; switch (propertyId) { - case CSSPropertyAll: - return valueID == CSSValueUnset; case CSSPropertyBackgroundRepeatX: // repeat | no-repeat case CSSPropertyBackgroundRepeatY: // repeat | no-repeat return valueID == CSSValueRepeat || valueID == CSSValueNoRepeat; @@ -461,7 +459,6 @@ bool isKeywordPropertyID(CSSPropertyID propertyId) switch (propertyId) { case CSSPropertyAlignItems: case CSSPropertyAlignSelf: - case CSSPropertyAll: case CSSPropertyBackgroundRepeatX: case CSSPropertyBackgroundRepeatY: case CSSPropertyBorderBottomStyle: diff --git a/engine/core/css/parser/CSSPropertyParser.cpp b/engine/core/css/parser/CSSPropertyParser.cpp index 2361012e06e..3a7502a5afc 100644 --- a/engine/core/css/parser/CSSPropertyParser.cpp +++ b/engine/core/css/parser/CSSPropertyParser.cpp @@ -5280,7 +5280,7 @@ PassRefPtr CSSPropertyParser::parseWillChange() ASSERT(CSSPropertyMetadata::isEnabledProperty(property)); // Now "all" is used by both CSSValue and CSSPropertyValue. // Need to return nullptr when currentValue is CSSPropertyAll. - if (property == CSSPropertyWillChange || property == CSSPropertyAll) + if (property == CSSPropertyWillChange) return nullptr; values->append(cssValuePool().createIdentifierValue(property)); } else { diff --git a/engine/core/css/resolver/StyleResolver.cpp b/engine/core/css/resolver/StyleResolver.cpp index ebb8229f65e..dfa466916aa 100644 --- a/engine/core/css/resolver/StyleResolver.cpp +++ b/engine/core/css/resolver/StyleResolver.cpp @@ -393,44 +393,6 @@ bool StyleResolver::isPropertyForPass(CSSPropertyID property) return firstCSSPropertyId() <= property && property <= lastCSSPropertyId(); } -// This method expands the 'all' shorthand property to longhand properties -// and applies the expanded longhand properties. -template -void StyleResolver::applyAllProperty(StyleResolverState& state, CSSValue* allValue) -{ - bool isUnsetValue = !allValue->isInitialValue() && !allValue->isInheritedValue(); - unsigned startCSSProperty = firstCSSPropertyId(); - unsigned endCSSProperty = lastCSSPropertyId(); - - for (unsigned i = startCSSProperty; i <= endCSSProperty; ++i) { - CSSPropertyID propertyId = static_cast(i); - - // StyleBuilder does not allow any expanded shorthands. - if (isExpandedShorthandForAll(propertyId)) - continue; - - // all shorthand spec says: - // The all property is a shorthand that resets all CSS properties - // except direction and unicode-bidi. - // c.f. http://dev.w3.org/csswg/css-cascade/#all-shorthand - // We skip applyProperty when a given property is unicode-bidi or - // direction. - if (!CSSProperty::isAffectedByAllProperty(propertyId)) - continue; - - CSSValue* value; - if (!isUnsetValue) { - value = allValue; - } else { - if (CSSPropertyMetadata::isInheritedProperty(propertyId)) - value = cssValuePool().createInheritedValue().get(); - else - value = cssValuePool().createExplicitInitialValue().get(); - } - StyleBuilder::applyProperty(propertyId, state, value); - } -} - template void StyleResolver::applyProperties(StyleResolverState& state, const StylePropertySet* properties, bool inheritedOnly) { @@ -438,12 +400,6 @@ void StyleResolver::applyProperties(StyleResolverState& state, const StyleProper for (unsigned i = 0; i < propertyCount; ++i) { StylePropertySet::PropertyReference current = properties->propertyAt(i); - CSSPropertyID property = current.id(); - if (property == CSSPropertyAll) { - applyAllProperty(state, current.value()); - continue; - } - if (inheritedOnly && !current.isInherited()) { // If the property value is explicitly inherited, we need to apply further non-inherited properties // as they might override the value inherited here. For this reason we don't allow declarations with @@ -452,6 +408,7 @@ void StyleResolver::applyProperties(StyleResolverState& state, const StyleProper continue; } + CSSPropertyID property = current.id(); if (!isPropertyForPass(property)) continue; if (pass == HighPriorityProperties && property == CSSPropertyLineHeight) diff --git a/engine/core/css/resolver/StyleResolver.h b/engine/core/css/resolver/StyleResolver.h index 5afa6cd03bd..5f55b00b9cf 100644 --- a/engine/core/css/resolver/StyleResolver.h +++ b/engine/core/css/resolver/StyleResolver.h @@ -121,8 +121,6 @@ private: void applyProperties(StyleResolverState&, const StylePropertySet* properties, bool inheritedOnly); template void applyAnimatedProperties(StyleResolverState&, const HashMap >&); - template - void applyAllProperty(StyleResolverState&, CSSValue*); MatchedPropertiesCache m_matchedPropertiesCache;