Title: [105898] trunk
Revision
105898
Author
alexis.men...@openbossa.org
Date
2012-01-25 10:54:40 -0800 (Wed, 25 Jan 2012)

Log Message

border-image should not crash when the source is not specified.
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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (105897 => 105898)


--- trunk/LayoutTests/ChangeLog	2012-01-25 18:38:34 UTC (rev 105897)
+++ trunk/LayoutTests/ChangeLog	2012-01-25 18:54:40 UTC (rev 105898)
@@ -1,3 +1,15 @@
+2012-01-25  Alexis Menard  <alexis.men...@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  <t...@chromium.org>
 
         Unreviewed, only skip scrollbars/scroll-rtl-or-bt-layer.html on qt-wk2.

Added: trunk/LayoutTests/fast/css/border-image-null-image-crash-expected.txt (0 => 105898)


--- trunk/LayoutTests/fast/css/border-image-null-image-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/css/border-image-null-image-crash-expected.txt	2012-01-25 18:54:40 UTC (rev 105898)
@@ -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
+

Added: trunk/LayoutTests/fast/css/border-image-null-image-crash.html (0 => 105898)


--- trunk/LayoutTests/fast/css/border-image-null-image-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/border-image-null-image-crash.html	2012-01-25 18:54:40 UTC (rev 105898)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+</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=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (105897 => 105898)


--- trunk/Source/WebCore/ChangeLog	2012-01-25 18:38:34 UTC (rev 105897)
+++ trunk/Source/WebCore/ChangeLog	2012-01-25 18:54:40 UTC (rev 105898)
@@ -1,3 +1,24 @@
+2012-01-25  Alexis Menard  <alexis.men...@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  <kenn...@webkit.org>
 
         [Qt] Implement tap feedback respecting -webkit-tap-highlight-color

Modified: trunk/Source/WebCore/css/CSSBorderImage.cpp (105897 => 105898)


--- trunk/Source/WebCore/css/CSSBorderImage.cpp	2012-01-25 18:38:34 UTC (rev 105897)
+++ trunk/Source/WebCore/css/CSSBorderImage.cpp	2012-01-25 18:54:40 UTC (rev 105898)
@@ -26,7 +26,8 @@
                                                 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();

Modified: trunk/Source/WebCore/css/CSSStyleSelector.cpp (105897 => 105898)


--- trunk/Source/WebCore/css/CSSStyleSelector.cpp	2012-01-25 18:38:34 UTC (rev 105897)
+++ trunk/Source/WebCore/css/CSSStyleSelector.cpp	2012-01-25 18:54:40 UTC (rev 105898)
@@ -4279,8 +4279,8 @@
 
 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,25 +4295,16 @@
     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) {
+        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 (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));
-            // Map in the image slices.
             if (slashList->item(0) && slashList->item(0)->isBorderImageSliceValue())
                 mapNinePieceImageSlice(slashList->item(0), image);
 
@@ -4324,9 +4315,9 @@
             // 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);
         }
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to