Demote 'line-height' to a low priority property.
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Jan 2015 17:52:35 +0000 (17:52 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Jan 2015 17:52:35 +0000 (17:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=140817

Reviewed by Andreas Kling.

Our special handling of the 'line-height' property is no longer
required, since the 'font' shorthand is now expanded in the parser
in all cases (also for system fonts).

This patch is based on the following Blink revision:
https://src.chromium.org/viewvc/blink?revision=184629&view=revision

No new tests, already covered by:
fast/css/font-shorthand-line-height.html
fast/css/line-height-font-order.html

* css/CSSPropertyNames.in:
* css/StyleResolver.cpp:
(WebCore::StyleResolver::styleForKeyframe):
(WebCore::StyleResolver::styleForPage):
(WebCore::StyleResolver::applyMatchedProperties):
(WebCore::StyleResolver::initializeFontStyle):
(WebCore::StyleResolver::CascadedProperties::Property::apply):
* css/StyleResolver.h:
(WebCore::StyleResolver::State::State):
(WebCore::StyleResolver::State::setLineHeightValue): Deleted.
(WebCore::StyleResolver::State::lineHeightValue): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179119 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/CSSPropertyNames.in
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h

index ecb6ed2..c34de5d 100644 (file)
@@ -1,3 +1,33 @@
+2015-01-26  Chris Dumez  <cdumez@apple.com>
+
+        Demote 'line-height' to a low priority property.
+        https://bugs.webkit.org/show_bug.cgi?id=140817
+
+        Reviewed by Andreas Kling.
+
+        Our special handling of the 'line-height' property is no longer
+        required, since the 'font' shorthand is now expanded in the parser
+        in all cases (also for system fonts).
+
+        This patch is based on the following Blink revision:
+        https://src.chromium.org/viewvc/blink?revision=184629&view=revision
+
+        No new tests, already covered by:
+        fast/css/font-shorthand-line-height.html
+        fast/css/line-height-font-order.html
+
+        * css/CSSPropertyNames.in:
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::styleForKeyframe):
+        (WebCore::StyleResolver::styleForPage):
+        (WebCore::StyleResolver::applyMatchedProperties):
+        (WebCore::StyleResolver::initializeFontStyle):
+        (WebCore::StyleResolver::CascadedProperties::Property::apply):
+        * css/StyleResolver.h:
+        (WebCore::StyleResolver::State::State):
+        (WebCore::StyleResolver::State::setLineHeightValue): Deleted.
+        (WebCore::StyleResolver::State::lineHeightValue): Deleted.
+
 2015-01-26  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [GTK] gtkdoc does not appear in DevHelp
index 207ef74..b603fc7 100644 (file)
@@ -119,13 +119,6 @@ text-rendering [Inherited, FontProperty, NameForMethods=TextRenderingMode]
 -epub-writing-mode = -webkit-writing-mode
 zoom [Custom=All]
 
-// line height needs to be right after the above high-priority properties
-#if defined(ENABLE_IOS_TEXT_AUTOSIZING) && ENABLE_IOS_TEXT_AUTOSIZING
-line-height [Inherited, Custom=All]
-#else
-line-height [Inherited, Getter=specifiedLineHeight, ConditionalConverter=LineHeight]
-#endif
-
 // Keep this in between the highest priority props and the lower ones.
 -webkit-ruby-position [Inherited]
 
@@ -244,6 +237,11 @@ kerning [Inherited, SVG, Converter=SVGLength]
 left [Initial=initialOffset, Converter=LengthOrAuto]
 letter-spacing [Inherited, Converter=Spacing]
 lighting-color [SVG, Converter=SVGColor]
+#if defined(ENABLE_IOS_TEXT_AUTOSIZING) && ENABLE_IOS_TEXT_AUTOSIZING
+line-height [Inherited, Custom=All]
+#else
+line-height [Inherited, Getter=specifiedLineHeight, ConditionalConverter=LineHeight]
+#endif
 list-style [Inherited, Longhands=list-style-type|list-style-position|list-style-image]
 list-style-image [Inherited, Converter=StyleImage<CSSPropertyListStyleImage>]
 list-style-position [Inherited]
index 8bc1109..f86b5be 100644 (file)
@@ -168,7 +168,8 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-static const CSSPropertyID firstLowPriorityProperty = static_cast<CSSPropertyID>(CSSPropertyLineHeight + 1);
+static const CSSPropertyID lastHighPriorityProperty = CSSPropertyZoom;
+static const CSSPropertyID firstLowPriorityProperty = static_cast<CSSPropertyID>(lastHighPriorityProperty + 1);
 
 class StyleResolver::CascadedProperties {
 public:
@@ -820,7 +821,6 @@ Ref<RenderStyle> StyleResolver::styleForKeyframe(const RenderStyle* elementStyle
     // Create the style
     state.setStyle(RenderStyle::clone(elementStyle));
     state.setParentStyle(RenderStyle::clone(elementStyle));
-    state.setLineHeightValue(0);
 
     TextDirection direction;
     WritingMode writingMode;
@@ -831,15 +831,11 @@ Ref<RenderStyle> StyleResolver::styleForKeyframe(const RenderStyle* elementStyle
     CascadedProperties cascade(direction, writingMode);
     cascade.addMatches(result, false, 0, result.matchedProperties.size() - 1);
 
-    applyCascadedProperties(cascade, firstCSSProperty, CSSPropertyLineHeight);
+    applyCascadedProperties(cascade, firstCSSProperty, lastHighPriorityProperty);
 
     // If our font got dirtied, go ahead and update it now.
     updateFont();
 
-    // Line-height is set when we are sure we decided on the font-size
-    if (state.lineHeightValue())
-        applyProperty(CSSPropertyLineHeight, state.lineHeightValue());
-
     // Now do rest of the properties.
     applyCascadedProperties(cascade, firstLowPriorityProperty, lastCSSProperty);
 
@@ -991,7 +987,6 @@ Ref<RenderStyle> StyleResolver::styleForPage(int pageIndex)
 
     PageRuleCollector collector(m_state, m_ruleSets);
     collector.matchAllPageRules(pageIndex);
-    m_state.setLineHeightValue(0);
 
     MatchResult& result = collector.matchedResult();
 
@@ -1002,15 +997,11 @@ Ref<RenderStyle> StyleResolver::styleForPage(int pageIndex)
     CascadedProperties cascade(direction, writingMode);
     cascade.addMatches(result, false, 0, result.matchedProperties.size() - 1);
 
-    applyCascadedProperties(cascade, firstCSSProperty, CSSPropertyLineHeight);
+    applyCascadedProperties(cascade, firstCSSProperty, lastHighPriorityProperty);
 
     // If our font got dirtied, go ahead and update it now.
     updateFont();
 
-    // Line-height is set when we are sure we decided on the font-size.
-    if (m_state.lineHeightValue())
-        applyProperty(CSSPropertyLineHeight, m_state.lineHeightValue());
-
     applyCascadedProperties(cascade, firstLowPriorityProperty, lastCSSProperty);
 
     cascade.applyDeferredProperties(*this);
@@ -1751,7 +1742,6 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
         // Find out if there's a -webkit-appearance property in effect from the UA sheet.
         // If so, we cache the border and background styles so that RenderTheme::adjustStyle()
         // can look at them later to figure out if this is a styled form control or not.
-        state.setLineHeightValue(nullptr);
         CascadedProperties cascade(direction, writingMode);
         if (!cascade.addMatches(matchResult, false, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly)
             || !cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly))
@@ -1761,7 +1751,7 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
         adjustStyleForInterCharacterRuby();
 
         // Start by applying properties that other properties may depend on.
-        applyCascadedProperties(cascade, firstCSSProperty, CSSPropertyLineHeight);
+        applyCascadedProperties(cascade, firstCSSProperty, lastHighPriorityProperty);
     
         updateFont();
         applyCascadedProperties(cascade, firstLowPriorityProperty, lastCSSProperty);
@@ -1776,15 +1766,13 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
         || !cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly))
         return applyMatchedProperties(matchResult, element, DoNotUseMatchedPropertiesCache);
 
-    state.setLineHeightValue(nullptr);
-
     applyCascadedProperties(cascade, CSSPropertyWebkitRubyPosition, CSSPropertyWebkitRubyPosition);
     
     // Adjust the font size to be smaller if ruby-position is inter-character.
     adjustStyleForInterCharacterRuby();
 
     // Start by applying properties that other properties may depend on.
-    applyCascadedProperties(cascade, firstCSSProperty, CSSPropertyLineHeight);
+    applyCascadedProperties(cascade, firstCSSProperty, lastHighPriorityProperty);
 
     // If the effective zoom value changes, we can't use the matched properties cache. Start over.
     if (cacheItem && cacheItem->renderStyle->effectiveZoom() != state.style()->effectiveZoom())
@@ -1793,10 +1781,6 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
     // If our font got dirtied, go ahead and update it now.
     updateFont();
 
-    // Line-height is set when we are sure we decided on the font-size.
-    if (state.lineHeightValue())
-        applyProperty(CSSPropertyLineHeight, state.lineHeightValue());
-
     // If the font changed, we can't use the matched properties cache. Start over.
     if (cacheItem && cacheItem->renderStyle->fontDescription() != state.style()->fontDescription())
         return applyMatchedProperties(matchResult, element, DoNotUseMatchedPropertiesCache);
@@ -2099,8 +2083,6 @@ void StyleResolver::initializeFontStyle(Settings* settings)
     fontDescription.setOneFamily(standardFamily);
     fontDescription.setKeywordSizeFromIdentifier(CSSValueMedium);
     setFontSize(fontDescription, Style::fontSizeForKeyword(CSSValueMedium, false, document()));
-    m_state.style()->setLineHeight(RenderStyle::initialLineHeight());
-    m_state.setLineHeightValue(0);
     setFontDescription(fontDescription);
 }
 
@@ -2688,13 +2670,6 @@ void StyleResolver::CascadedProperties::Property::apply(StyleResolver& resolver)
 {
     State& state = resolver.state();
 
-    // FIXME: It would be nice if line-height were less of a special snowflake.
-    if (id == CSSPropertyLineHeight) {
-        if (auto value = state.style()->insideLink() == NotInsideLink ? cssValue[0] : cssValue[SelectorChecker::MatchLink])
-            state.setLineHeightValue(value);
-        return;
-    }
-
     if (cssValue[0]) {
         state.setApplyPropertyToRegularStyle(true);
         state.setApplyPropertyToVisitedLinkStyle(false);
index 7338532..ab65ede 100644 (file)
@@ -344,7 +344,6 @@ public:
             , m_elementAffectedByClassRules(false)
             , m_applyPropertyToRegularStyle(true)
             , m_applyPropertyToVisitedLinkStyle(false)
-            , m_lineHeightValue(nullptr)
             , m_fontDirty(false)
             , m_fontSizeHasViewportUnits(false)
             , m_hasUAAppearance(false)
@@ -382,8 +381,6 @@ public:
         Vector<RefPtr<ReferenceFilterOperation>>& filtersWithPendingSVGDocuments() { return m_filtersWithPendingSVGDocuments; }
         Vector<RefPtr<MaskImageOperation>>& maskImagesWithPendingSVGDocuments() { return m_maskImagesWithPendingSVGDocuments; }
 
-        void setLineHeightValue(CSSValue* value) { m_lineHeightValue = value; }
-        CSSValue* lineHeightValue() { return m_lineHeightValue; }
         void setFontDirty(bool isDirty) { m_fontDirty = isDirty; }
         bool fontDirty() const { return m_fontDirty; }
         void setFontSizeHasViewportUnits(bool hasViewportUnits) { m_fontSizeHasViewportUnits = hasViewportUnits; }
@@ -431,7 +428,6 @@ public:
         Vector<RefPtr<ReferenceFilterOperation>> m_filtersWithPendingSVGDocuments;
         Vector<RefPtr<MaskImageOperation>> m_maskImagesWithPendingSVGDocuments;
 
-        CSSValue* m_lineHeightValue;
         bool m_fontDirty;
         bool m_fontSizeHasViewportUnits;