When no background-size is specified on the 2nd background layer, it takes the first...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Jan 2015 22:54:34 +0000 (22:54 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Jan 2015 22:54:34 +0000 (22:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141059

Reviewed by Antti Koivisto.

Source/WebCore:

This patch fixes fill size support for 'initial' value.

Test: fast/css/background-layers-initial-size.html

* css/CSSToStyleMap.cpp:
- Add check for initial values and set fill size to initialFillSize()
  in this case (which is 'auto'). Previously, we were handling all
  non CSSPrimitiveValues the same way and setting the fill size type
  to SizeNone, which means no size (not 'auto').
- Clean up the rest of the function (no behavior change).

* rendering/style/FillLayer.cpp:
(WebCore::FillLayer::FillLayer):
- Initialize m_sizeLength to SizeNone instead of calling
  initialFillSizeType(). There is no behavior change here. However,
  initialFillSizeType() was not supposed to return SizeNone.
- Stop explicitly initializing m_sizeLength to LengthSize() as this
  is already what happens implicitly.

* rendering/style/FillLayer.h:
(WebCore::FillLayer::initialFillSize):
Return FillSize() instead of FillSize(SizeNone, LengthSize()).
FillSize() is equivalent to FillSize(SizeLength, LengthSize())
which is resolved to 'auto'. SizeNone means no size which isn't
what we want as an initial value.

(WebCore::FillLayer::initialFillSizeType): Deleted.
(WebCore::FillLayer::initialFillSizeLength): Deleted.
Remove Individual initialFillSizeType() / initialFillSizeLength()
functions now that all caller use initialFillSize() instead.

LayoutTests:

Add layout test to cover the case where we have 2 background layers, with an explicit
size only for the first one.

* fast/css/background-layers-initial-size-expected.txt: Added.
* fast/css/background-layers-initial-size.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/background-layers-initial-size-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/background-layers-initial-size.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSToStyleMap.cpp
Source/WebCore/rendering/style/FillLayer.cpp
Source/WebCore/rendering/style/FillLayer.h

index 2a95258..de5e990 100644 (file)
@@ -1,3 +1,16 @@
+2015-01-30  Chris Dumez  <cdumez@apple.com>
+
+        When no background-size is specified on the 2nd background layer, it takes the first instead of the initial value
+        https://bugs.webkit.org/show_bug.cgi?id=141059
+
+        Reviewed by Antti Koivisto.
+
+        Add layout test to cover the case where we have 2 background layers, with an explicit
+        size only for the first one.
+
+        * fast/css/background-layers-initial-size-expected.txt: Added.
+        * fast/css/background-layers-initial-size.html: Added.
+
 2015-01-30  Matthew Mirman  <mmirman@apple.com>
 
         Added a test for JSON.stringify on ClientRect.
diff --git a/LayoutTests/fast/css/background-layers-initial-size-expected.txt b/LayoutTests/fast/css/background-layers-initial-size-expected.txt
new file mode 100644 (file)
index 0000000..16d6acc
--- /dev/null
@@ -0,0 +1,10 @@
+Test that 'initial' background-size values are properly handled in background layers.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS computedBackgroundSize is "25px 25px, auto"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/css/background-layers-initial-size.html b/LayoutTests/fast/css/background-layers-initial-size.html
new file mode 100644 (file)
index 0000000..43d8e38
--- /dev/null
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<style>
+div {
+    background: linear-gradient(45deg, rgba(0,0,0,.4) 50%, transparent 0) 100% 0 / 25px 25px no-repeat,
+                linear-gradient(-135deg, transparent 18px, yellowgreen 0);
+
+    width: 12em;
+    height: 8em;
+}
+</style>
+</head>
+<body>
+<div id="testDiv"></div>
+<script>
+description("Test that 'initial' background-size values are properly handled in background layers.");
+
+var computedBackgroundSize = window.getComputedStyle(testDiv)["background-size"];
+// The second layer should have "auto" as background size as it is its initial value.
+shouldBeEqualToString("computedBackgroundSize", "25px 25px, auto");
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 3a39609..18d40b3 100644 (file)
@@ -1,3 +1,41 @@
+2015-01-30  Chris Dumez  <cdumez@apple.com>
+
+        When no background-size is specified on the 2nd background layer, it takes the first instead of the initial value
+        https://bugs.webkit.org/show_bug.cgi?id=141059
+
+        Reviewed by Antti Koivisto.
+
+        This patch fixes fill size support for 'initial' value.
+
+        Test: fast/css/background-layers-initial-size.html
+
+        * css/CSSToStyleMap.cpp:
+        - Add check for initial values and set fill size to initialFillSize()
+          in this case (which is 'auto'). Previously, we were handling all
+          non CSSPrimitiveValues the same way and setting the fill size type
+          to SizeNone, which means no size (not 'auto').
+        - Clean up the rest of the function (no behavior change).
+
+        * rendering/style/FillLayer.cpp:
+        (WebCore::FillLayer::FillLayer):
+        - Initialize m_sizeLength to SizeNone instead of calling
+          initialFillSizeType(). There is no behavior change here. However,
+          initialFillSizeType() was not supposed to return SizeNone.
+        - Stop explicitly initializing m_sizeLength to LengthSize() as this
+          is already what happens implicitly.
+
+        * rendering/style/FillLayer.h:
+        (WebCore::FillLayer::initialFillSize):
+        Return FillSize() instead of FillSize(SizeNone, LengthSize()).
+        FillSize() is equivalent to FillSize(SizeLength, LengthSize())
+        which is resolved to 'auto'. SizeNone means no size which isn't
+        what we want as an initial value.
+
+        (WebCore::FillLayer::initialFillSizeType): Deleted.
+        (WebCore::FillLayer::initialFillSizeLength): Deleted.
+        Remove Individual initialFillSizeType() / initialFillSizeLength()
+        functions now that all caller use initialFillSize() instead.
+
 2015-01-30  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r179403.
index 0719abd..dc61a5c 100644 (file)
@@ -184,47 +184,43 @@ void CSSToStyleMap::mapFillRepeatY(CSSPropertyID, FillLayer& layer, CSSValue& va
     layer.setRepeatY(downcast<CSSPrimitiveValue>(value));
 }
 
-void CSSToStyleMap::mapFillSize(CSSPropertyID, FillLayer& layer, CSSValue& value)
+static inline bool convertToLengthSize(const CSSPrimitiveValue& primitiveValue, CSSToLengthConversionData conversionData, LengthSize& size)
 {
-    if (!is<CSSPrimitiveValue>(value)) {
-        layer.setSizeType(SizeNone);
-        return;
-    }
-
-    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
-    if (primitiveValue.getValueID() == CSSValueContain)
-        layer.setSizeType(Contain);
-    else if (primitiveValue.getValueID() == CSSValueCover)
-        layer.setSizeType(Cover);
-    else
-        layer.setSizeType(SizeLength);
+    if (auto* pair = primitiveValue.getPairValue()) {
+        size.setWidth(pair->first()->convertToLength<AnyConversion>(conversionData));
+        size.setHeight(pair->second()->convertToLength<AnyConversion>(conversionData));
+    } else
+        size.setWidth(primitiveValue.convertToLength<AnyConversion>(conversionData));
 
-    LengthSize b = FillLayer::initialFillSizeLength(layer.type());
+    return !size.width().isUndefined() && !size.height().isUndefined();
+}
 
-    if (value.isInitialValue() || primitiveValue.getValueID() == CSSValueContain || primitiveValue.getValueID() == CSSValueCover) {
-        layer.setSizeLength(b);
+void CSSToStyleMap::mapFillSize(CSSPropertyID, FillLayer& layer, CSSValue& value)
+{
+    if (value.isInitialValue()) {
+        layer.setSize(FillLayer::initialFillSize(layer.type()));
         return;
     }
 
-    Length firstLength;
-    Length secondLength;
-
-    if (Pair* pair = primitiveValue.getPairValue()) {
-        CSSPrimitiveValue* first = static_cast<CSSPrimitiveValue*>(pair->first());
-        CSSPrimitiveValue* second = static_cast<CSSPrimitiveValue*>(pair->second());
-        firstLength = first->convertToLength<AnyConversion>(m_resolver->state().cssToLengthConversionData());
-        secondLength = second->convertToLength<AnyConversion>(m_resolver->state().cssToLengthConversionData());
-    } else {
-        firstLength = primitiveValue.convertToLength<AnyConversion>(m_resolver->state().cssToLengthConversionData());
-        secondLength = Length();
-    }
-
-    if (firstLength.isUndefined() || secondLength.isUndefined())
+    if (!is<CSSPrimitiveValue>(value))
         return;
 
-    b.setWidth(firstLength);
-    b.setHeight(secondLength);
-    layer.setSizeLength(b);
+    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
+    FillSize fillSize;
+    switch (primitiveValue.getValueID()) {
+    case CSSValueContain:
+        fillSize.type = Contain;
+        break;
+    case CSSValueCover:
+        fillSize.type = Cover;
+        break;
+    default:
+        ASSERT(fillSize.type == SizeLength);
+        if (!convertToLengthSize(primitiveValue, m_resolver->state().cssToLengthConversionData(), fillSize.size))
+            return;
+        break;
+    }
+    layer.setSize(fillSize);
 }
 
 void CSSToStyleMap::mapFillXPosition(CSSPropertyID propertyID, FillLayer& layer, CSSValue& value)
index fc11593..3c5196a 100644 (file)
@@ -45,14 +45,13 @@ FillLayer::FillLayer(EFillLayerType type)
     : m_image(FillLayer::initialFillImage(type))
     , m_xPosition(FillLayer::initialFillXPosition(type))
     , m_yPosition(FillLayer::initialFillYPosition(type))
-    , m_sizeLength(FillLayer::initialFillSizeLength(type))
     , m_attachment(FillLayer::initialFillAttachment(type))
     , m_clip(FillLayer::initialFillClip(type))
     , m_origin(FillLayer::initialFillOrigin(type))
     , m_repeatX(FillLayer::initialFillRepeatX(type))
     , m_repeatY(FillLayer::initialFillRepeatY(type))
     , m_composite(FillLayer::initialFillComposite(type))
-    , m_sizeType(FillLayer::initialFillSizeType(type))
+    , m_sizeType(SizeNone)
     , m_blendMode(FillLayer::initialFillBlendMode(type))
     , m_maskSourceType(FillLayer::initialFillMaskSourceType(type))
     , m_imageSet(false)
index 2f840f0..73e6649 100644 (file)
@@ -166,12 +166,10 @@ public:
     static EFillRepeat initialFillRepeatY(EFillLayerType) { return RepeatFill; }
     static CompositeOperator initialFillComposite(EFillLayerType) { return CompositeSourceOver; }
     static BlendMode initialFillBlendMode(EFillLayerType) { return BlendModeNormal; }
-    static EFillSizeType initialFillSizeType(EFillLayerType) { return SizeNone; }
-    static LengthSize initialFillSizeLength(EFillLayerType) { return LengthSize(); }
-    static FillSize initialFillSize(EFillLayerType type) { return FillSize(initialFillSizeType(type), initialFillSizeLength(type)); }
+    static FillSize initialFillSize(EFillLayerType) { return FillSize(); }
     static Length initialFillXPosition(EFillLayerType) { return Length(0.0f, Percent); }
     static Length initialFillYPosition(EFillLayerType) { return Length(0.0f, Percent); }
-    static StyleImage* initialFillImage(EFillLayerType) { return 0; }
+    static StyleImage* initialFillImage(EFillLayerType) { return nullptr; }
     static EMaskSourceType initialFillMaskSourceType(EFillLayerType) { return MaskAlpha; }
 
 private: