Changeset 157598 in webkit

Timestamp:

Oct 17, 2013 2:14:46 PM (11 years ago)

Author:

thakis@chromium.org

Message:

Fix three bugs in the equals() implementations for css gradients.
​https://bugs.webkit.org/show_bug.cgi?id=122987

Reviewed by Andreas Kling.

1. Linear gradients were considered equal if the first gradient has no x and y position and the second has no x but does have y.
2. Same as 1, for radial gradients. (This doesn't happen in practice as CSSParser::parseRadialGradient rejects such input, so no test for this case.)
3. Radial gradients without x and y position weren't considered equal even if they were.

Source/WebCore:

• css/CSSGradientValue.cpp:

(WebCore::CSSLinearGradientValue::equals):
(WebCore::CSSRadialGradientValue::equals):

LayoutTests:

Merges ​https://codereview.chromium.org/26147006/

• cssom/cssvalue-comparison-expected.txt:
• cssom/cssvalue-comparison.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/cssom/cssvalue-comparison-expected.txt (modified) (1 diff)
• LayoutTests/cssom/cssvalue-comparison.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSGradientValue.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-10-17  Nico Weber  <thakis@chromium.org>
+
+        Fix three bugs in the equals() implementations for css gradients.
+        https://bugs.webkit.org/show_bug.cgi?id=122987
+
+        Reviewed by Andreas Kling.
+
+        1. Linear gradients were considered equal if the first gradient has no x and y
+           position and the second has no x but does have y.
+        2. Same as 1, for radial gradients. (This doesn't happen in practice as
+           CSSParser::parseRadialGradient rejects such input, so no test for this case.)
+        3. Radial gradients without x and y position weren't considered equal even if
+           they were.
+
+        Merges https://codereview.chromium.org/26147006/
+
+        * cssom/cssvalue-comparison-expected.txt:
+        * cssom/cssvalue-comparison.html:
+
 2013-10-17  Roger Fong  <roger_fong@apple.com>
 

trunk/LayoutTests/cssom/cssvalue-comparison-expected.txt

@@ -67 +67 @@
 PASS Two CSSValues "-webkit-gradient(radial, 45 45, 0, 52 50, 0, from(#A7D30C), to(rgba(1,159,98,0)), color-stop(90%, #019F62))" for property "background" are equal. 
 PASS Two CSSValues "-webkit-gradient(linear, left top, left bottom, from(#ccc), to(#000))" and "-webkit-gradient(radial, 45 45, 0, 52 50, 0, from(#A7D30C), to(rgba(1,159,98,0)), color-stop(90%, #019F62))" for property "background" are not equal. 
+PASS Two CSSValues "radial-gradient(circle, #ccc, #000)" for property "background" are equal. 
+PASS Two CSSValues "linear-gradient(#000, #234)" for property "background" are equal. 
+PASS Two CSSValues "linear-gradient(to top, #000, #234)" for property "background" are equal. 
+PASS Two CSSValues "linear-gradient(#000, #234)" and "linear-gradient(to top, #000, #234)" for property "background" are not equal. 
 PASS Two CSSValues "-webkit-cross-fade(url(dummy://example.png), url(dummy://example.png), 50%)" for property "background-image" are equal. 
 PASS Two CSSValues "-webkit-cross-fade(url(dummy://background.png), url(dummy://foreground.png), 80%)" for property "background-image" are equal. 

trunk/LayoutTests/cssom/cssvalue-comparison.html

@@ -51 +51 @@
                   {"font" : ["italic bold 12px/30px arial", "italic bold 8px/16px helvetica"]}, // font
                   {"background" : ["-webkit-gradient(linear, left top, left bottom, from(#ccc), to(#000))", "-webkit-gradient(radial, 45 45, 0, 52 50, 0, from(#A7D30C), to(rgba(1,159,98,0)), color-stop(90%, #019F62))"]}, // gradients
+                  {"background" : ["radial-gradient(circle, #ccc, #000)"]}, // gradients
+                  {"background" : ["linear-gradient(#000, #234)", "linear-gradient(to top, #000, #234)"]}, // gradients
                   {"background-image" : ["-webkit-cross-fade(url(dummy://example.png), url(dummy://example.png), 50%)", "-webkit-cross-fade(url(dummy://background.png), url(dummy://foreground.png), 80%)"]}, // crossfade
                   {"-webkit-box-reflect" : ["below 10px", "below 0px -webkit-gradient(linear, left top, left bottom, from(transparent), to(rgba(10, 55, 234, 1)))"]}, // reflect

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-10-17  Nico Weber  <thakis@chromium.org>
+
+        Fix three bugs in the equals() implementations for css gradients.
+        https://bugs.webkit.org/show_bug.cgi?id=122987
+
+        Reviewed by Andreas Kling.
+
+        1. Linear gradients were considered equal if the first gradient has no x and y
+           position and the second has no x but does have y.
+        2. Same as 1, for radial gradients. (This doesn't happen in practice as
+           CSSParser::parseRadialGradient rejects such input, so no test for this case.)
+        3. Radial gradients without x and y position weren't considered equal even if
+           they were.
+
+        * css/CSSGradientValue.cpp:
+        (WebCore::CSSLinearGradientValue::equals):
+        (WebCore::CSSRadialGradientValue::equals):
+
 2013-10-17  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/css/CSSGradientValue.cpp

@@ -727 +727 @@
         return false;
 
-    bool equalXorY = false;
+    bool equalXandY = false;
     if (m_firstX && m_firstY)
-        equalXorY = compareCSSValuePtr(m_firstX, other.m_firstX) && compareCSSValuePtr(m_firstY, other.m_firstY);
+        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && compareCSSValuePtr(m_firstY, other.m_firstY);
     else if (m_firstX)
-        equalXorY =compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY;
+        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY;
     else if (m_firstY)
-        equalXorY = compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX;
+        equalXandY = compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX;
     else
-        equalXorY = !other.m_firstX || !other.m_firstY;
-
-    return equalXorY && m_stops == other.m_stops;
+        equalXandY = !other.m_firstX && !other.m_firstY;
+
+    return equalXandY && m_stops == other.m_stops;
 }
 
@@ -1139 +1139 @@
         return false;
 
-    bool equalXorY = false;
+    bool equalXandY = false;
     if (m_firstX && m_firstY)
-        equalXorY = compareCSSValuePtr(m_firstX, other.m_firstX) && compareCSSValuePtr(m_firstY, other.m_firstY);
+        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && compareCSSValuePtr(m_firstY, other.m_firstY);
     else if (m_firstX)
-        equalXorY = compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY;
+        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY;
     else if (m_firstY)
-        equalXorY = compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX;
+        equalXandY = compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX;
     else
-        equalXorY == !other.m_firstX || !other.m_firstY;
-
-    if (!equalXorY)
+        equalXandY = !other.m_firstX && !other.m_firstY;
+
+    if (!equalXandY)
         return false;