border-image should not crash when the source is not specified.
authoralexis.menard@openbossa.org <alexis.menard@openbossa.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jan 2012 18:54:40 +0000 (18:54 +0000)
committeralexis.menard@openbossa.org <alexis.menard@openbossa.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jan 2012 18:54:40 +0000 (18:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=77001

Reviewed by Andreas Kling.

Source/WebCore:

This bug was introduced by r105502 but was exposed by r105738.
The image-source of a border-image is not mandatory therefore it
may happen that you have no value set for it. WebCore::createBorderImageValue
was wrongly assuming that the image is always set. This problem also required a bit
of refactoring in CSSStyleSelector::mapNinePieceImage to take into account that
the image could be optional (just like other properties).

Test: fast/css/border-image-null-image-crash.html

* css/CSSBorderImage.cpp:
(WebCore::createBorderImageValue):
* css/CSSStyleSelector.cpp:
(WebCore::CSSStyleSelector::mapNinePieceImage):

LayoutTests:

Add a new test to cover this crash specifically.

* fast/css/border-image-null-image-crash-expected.txt: Added.
* fast/css/border-image-null-image-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/border-image-null-image-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/border-image-null-image-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSBorderImage.cpp
Source/WebCore/css/CSSStyleSelector.cpp

index 86ea024..bf580bc 100644 (file)
@@ -1,3 +1,15 @@
+2012-01-25  Alexis Menard  <alexis.menard@openbossa.org>
+
+        border-image should not crash when the source is not specified.
+        https://bugs.webkit.org/show_bug.cgi?id=77001
+
+        Reviewed by Andreas Kling.
+
+        Add a new test to cover this crash specifically.
+
+        * fast/css/border-image-null-image-crash-expected.txt: Added.
+        * fast/css/border-image-null-image-crash.html: Added.
+
 2012-01-25  Tony Chang  <tony@chromium.org>
 
         Unreviewed, only skip scrollbars/scroll-rtl-or-bt-layer.html on qt-wk2.
diff --git a/LayoutTests/fast/css/border-image-null-image-crash-expected.txt b/LayoutTests/fast/css/border-image-null-image-crash-expected.txt
new file mode 100644 (file)
index 0000000..114abe1
--- /dev/null
@@ -0,0 +1,10 @@
+Tests that shorthand border-image with a null image doesn't crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS computedStyle.getPropertyValue('border-image') is 'none'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/css/border-image-null-image-crash.html b/LayoutTests/fast/css/border-image-null-image-crash.html
new file mode 100644 (file)
index 0000000..ce3ed58
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+
+description("Tests that shorthand border-image with a null image doesn't crash.");
+
+var testContainer = document.createElement("div");
+document.body.appendChild(testContainer);
+
+testContainer.innerHTML = '<div id="test">hello</div>';
+
+e = document.getElementById('test');
+computedStyle = window.getComputedStyle(e, null);
+e.style.borderImage = "10% fill";
+
+shouldBe("computedStyle.getPropertyValue('border-image')", "'none'");
+
+document.body.removeChild(testContainer);
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 7333859..301729a 100644 (file)
@@ -1,3 +1,24 @@
+2012-01-25  Alexis Menard  <alexis.menard@openbossa.org>
+
+        border-image should not crash when the source is not specified.
+        https://bugs.webkit.org/show_bug.cgi?id=77001
+
+        Reviewed by Andreas Kling.
+
+        This bug was introduced by r105502 but was exposed by r105738.
+        The image-source of a border-image is not mandatory therefore it
+        may happen that you have no value set for it. WebCore::createBorderImageValue
+        was wrongly assuming that the image is always set. This problem also required a bit
+        of refactoring in CSSStyleSelector::mapNinePieceImage to take into account that
+        the image could be optional (just like other properties).
+
+        Test: fast/css/border-image-null-image-crash.html
+
+        * css/CSSBorderImage.cpp:
+        (WebCore::createBorderImageValue):
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::mapNinePieceImage):
+
 2012-01-25  Kenneth Rohde Christiansen  <kenneth@webkit.org>
 
         [Qt] Implement tap feedback respecting -webkit-tap-highlight-color
index 3b66c6b..ac3e51e 100644 (file)
@@ -26,7 +26,8 @@ PassRefPtr<CSSValueList> createBorderImageValue(PassRefPtr<CSSValue> image, Pass
                                                 PassRefPtr<CSSValue> outset, PassRefPtr<CSSValue> repeat)
 {
     RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
-    list->append(image);
+    if (image)
+        list->append(image);
 
     if (borderSlice || outset) {
         RefPtr<CSSValueList> listSlash = CSSValueList::createSlashSeparated();
index 8d44ca6..6034073 100644 (file)
@@ -4279,8 +4279,8 @@ void CSSStyleSelector::mapAnimationTimingFunction(Animation* animation, CSSValue
 
 void CSSStyleSelector::mapNinePieceImage(CSSPropertyID property, CSSValue* value, NinePieceImage& image)
 {
-    // If we're a primitive value, then we are "none" and don't need to alter the empty image at all.
-    if (!value || value->isPrimitiveValue())
+    // If we're not a value list, then we are "none" and don't need to alter the empty image at all.
+    if (!value || !value->isValueList())
         return;
 
     // Retrieve the border image value.
@@ -4295,24 +4295,15 @@ void CSSStyleSelector::mapNinePieceImage(CSSPropertyID property, CSSValue* value
     else
         imageProperty = property;
 
-    if (CSSValue* imageValue = borderImage->item(0))
-        image.setImage(styleImage(imageProperty, imageValue));
+    for (unsigned i = 0 ; i < borderImage->length() ; ++i) {
+        CSSValue* current = borderImage->item(i);
 
-    if (borderImage->item(1)) {
-        if (borderImage->item(1)->cssValueType() != CSSValue::CSS_VALUE_LIST) {
-            // Map in the image slices.
-            if (borderImage->item(1)) {
-                if (borderImage->item(1)->isBorderImageSliceValue()) {
-                    mapNinePieceImageSlice(borderImage->item(1), image);
-                     if (borderImage->item(2))
-                        // Set the appropriate rules for stretch/round/repeat of the slices
-                        mapNinePieceImageRepeat(borderImage->item(2), image);
-                } else
-                    // Set the appropriate rules for stretch/round/repeat of the slices
-                    mapNinePieceImageRepeat(borderImage->item(1), image);
-            }
-        } else {
-            CSSValueList* slashList = static_cast<CSSValueList*>(borderImage->item(1));
+        if (current->isImageValue() || current->isImageGeneratorValue())
+            image.setImage(styleImage(imageProperty, current));
+        else if (current->isBorderImageSliceValue())
+            mapNinePieceImageSlice(current, image);
+        else if (current->isValueList()) {
+            CSSValueList* slashList = static_cast<CSSValueList*>(current);
             // Map in the image slices.
             if (slashList->item(0) && slashList->item(0)->isBorderImageSliceValue())
                 mapNinePieceImageSlice(slashList->item(0), image);
@@ -4324,9 +4315,9 @@ void CSSStyleSelector::mapNinePieceImage(CSSPropertyID property, CSSValue* value
             // Map in the outset.
             if (slashList->item(2))
                 image.setOutset(mapNinePieceImageQuad(slashList->item(2)));
-
-            // Set the appropriate rules for stretch/round/repeat of the slices
-            mapNinePieceImageRepeat(borderImage->item(2), image);
+        } else if (current->isPrimitiveValue()) {
+            // Set the appropriate rules for stretch/round/repeat of the slices.
+            mapNinePieceImageRepeat(current, image);
         }
     }