Changeset 183765 in webkit


Ignore:
Timestamp:
May 4, 2015 1:27:50 PM (9 years ago)
Author:
Chris Dumez
Message:

REGRESSION (r178156): CSS Parser incorrectly rejects valid calc() in padding-right property
​https://bugs.webkit.org/show_bug.cgi?id=144584
<rdar://problem/20796829>

Reviewed by Darin Adler.

Source/WebCore:

The CSS parser was rejecting calculated values at parsing time if it
considered the value was negative and the CSS property did not allow
negative values. However, doing so at this point will not always work
because we don't necessarily know the font-size yet (for e.g. for
calc(0.5em - 2px). Also, rejecting negative calculated values is not
the right behavior as the the specification. The specification says
we should clamp:
​http://dev.w3.org/csswg/css-values-3/#calc-range

This patch updates validateCalculationUnit() to stop marking the value
as invalid if it is negative. Instead, let the CSSCalcValue's permitted
range clamp the value as needed.

This bug was causing the bottom graphic on aldentrio.com to not be
rendered properly.

Test: fast/css/negative-calc-values.html

fast/css/padding-calc-value.html

  • css/CSSParser.cpp:

(WebCore::CSSParser::validateCalculationUnit):

LayoutTests:

  • fast/css/negative-calc-values-expected.txt: Added.
  • fast/css/negative-calc-values.html: Added.

Add a layout test that assigns negative calc() values to properties
whose values cannot be negative to verify that values are clamped as
per the specification:
​http://dev.w3.org/csswg/css-values-3/#calc-range

  • fast/css/padding-calc-value-expected.txt: Added.
  • fast/css/padding-calc-value.html: Added.

Add a layout test to test that using calc(.5em - 2px) for padding-right
CSS property works as intended. It used to be resolved as 0px instead
of "2*font-size - 2px".

  • fast/css/text-shadow-calc-value-expected.txt:
  • fast/css/text-shadow-calc-value.html:

Update test to match what the specification says:
​http://dev.w3.org/csswg/css-values-3/#calc-range
"width: calc(5px - 10px);" is equivalent to "width: 0px;" since widths
smaller than 0px are not allowed.

Location:
trunk
Files:
4 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r183761 r183765  
     12015-05-04  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION (r178156): CSS Parser incorrectly rejects valid calc() in padding-right property
     4        https://bugs.webkit.org/show_bug.cgi?id=144584
     5        <rdar://problem/20796829>
     6
     7        Reviewed by Darin Adler.
     8
     9        * fast/css/negative-calc-values-expected.txt: Added.
     10        * fast/css/negative-calc-values.html: Added.
     11        Add a layout test that assigns negative calc() values to properties
     12        whose values cannot be negative to verify that values are clamped as
     13        per the specification:
     14        http://dev.w3.org/csswg/css-values-3/#calc-range
     15
     16        * fast/css/padding-calc-value-expected.txt: Added.
     17        * fast/css/padding-calc-value.html: Added.
     18        Add a layout test to test that using calc(.5em - 2px) for padding-right
     19        CSS property works as intended. It used to be resolved as 0px instead
     20        of "2*font-size - 2px".
     21
     22        * fast/css/text-shadow-calc-value-expected.txt:
     23        * fast/css/text-shadow-calc-value.html:
     24        Update test to match what the specification says:
     25        http://dev.w3.org/csswg/css-values-3/#calc-range
     26        "width: calc(5px - 10px);" is equivalent to "width: 0px;" since widths
     27        smaller than 0px are not allowed.
     28
    1292015-05-04  Joseph Pecoraro  <pecoraro@apple.com>
    230

  • trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt

    r178156 r183765  
    1212PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is "rgb(255, 255, 255) -3px -6px 9px"
    1313testDiv.style['text-shadow'] = 'calc(1 * 3px) calc(2 * 3px) calc(-3 * 3px) rgb(255, 255, 255)'
    14 PASS testDiv.style['text-shadow'] is previousStyle
    15 PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is previousComputedStyle
     14PASS testDiv.style['text-shadow'] is "rgb(255, 255, 255) calc(3px) calc(6px) calc(-9px)"
     15PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is "rgb(255, 255, 255) 3px 6px 0px"
    1616PASS successfullyParsed is true
    1717

  • trunk/LayoutTests/fast/css/text-shadow-calc-value.html

    r178156 r183765  
    1818shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "rgb(255, 255, 255) -3px -6px 9px")
    1919
    20 var previousStyle = testDiv.style['text-shadow'];
    21 var previousComputedStyle = window.getComputedStyle(testDiv).getPropertyValue('text-shadow');
    22 
    23 // Negative blur-radius is not allowed.
     20// Negative blur-radius is not allowed so it should become 0.
    2421evalAndLog("testDiv.style['text-shadow'] = 'calc(1 * 3px) calc(2 * 3px) calc(-3 * 3px) rgb(255, 255, 255)'");
    2522// text-shadow should not be updated.
    26 shouldBe("testDiv.style['text-shadow']", "previousStyle");
    27 shouldBe("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "previousComputedStyle")
     23shouldBeEqualToString("testDiv.style['text-shadow']", "rgb(255, 255, 255) calc(3px) calc(6px) calc(-9px)");
     24shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "rgb(255, 255, 255) 3px 6px 0px")
    2825
    2926</script>

  • trunk/Source/WebCore/ChangeLog

    r183751 r183765  
     12015-05-04  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION (r178156): CSS Parser incorrectly rejects valid calc() in padding-right property
     4        https://bugs.webkit.org/show_bug.cgi?id=144584
     5        <rdar://problem/20796829>
     6
     7        Reviewed by Darin Adler.
     8
     9        The CSS parser was rejecting calculated values at parsing time if it
     10        considered the value was negative and the CSS property did not allow
     11        negative values. However, doing so at this point will not always work
     12        because we don't necessarily know the font-size yet (for e.g. for
     13        calc(0.5em - 2px). Also, rejecting negative calculated values is not
     14        the right behavior as the the specification. The specification says
     15        we should clamp:
     16        http://dev.w3.org/csswg/css-values-3/#calc-range
     17
     18        This patch updates validateCalculationUnit() to stop marking the value
     19        as invalid if it is negative. Instead, let the CSSCalcValue's permitted
     20        range clamp the value as needed.
     21
     22        This bug was causing the bottom graphic on aldentrio.com to not be
     23        rendered properly.
     24
     25        Test: fast/css/negative-calc-values.html
     26              fast/css/padding-calc-value.html
     27
     28        * css/CSSParser.cpp:
     29        (WebCore::CSSParser::validateCalculationUnit):
     30
    1312015-05-04  Eric Carlson  <eric.carlson@apple.com>
    232

  • trunk/Source/WebCore/css/CSSCalculationValue.h

    r178627 r183765  
    9393    bool isInt() const { return m_expression->isInteger(); }
    9494    double doubleValue() const;
    95     bool isNegative() const { return m_expression->doubleValue() < 0; }
    9695    bool isPositive() const { return m_expression->doubleValue() > 0; }
    9796    double computeLengthPx(const CSSToLengthConversionData&) const;

  • trunk/Source/WebCore/css/CSSParser.cpp

    r183748 r183765  
    16711671        if (!isValid && (unitFlags & FPositiveInteger) && calculation->isInt() && calculation->isPositive())
    16721672            isValid = true;
    1673         if (isValid && mustBeNonNegative && calculation->isNegative())
    1674             isValid = false;
    16751673        break;
    16761674    case CalcLength:
    16771675        isValid = (unitFlags & FLength);
    1678         if (isValid && mustBeNonNegative && calculation->isNegative())
    1679             isValid = false;
    16801676        break;
    16811677    case CalcPercent:
    16821678        isValid = (unitFlags & FPercent);
    1683         if (isValid && mustBeNonNegative && calculation->isNegative())
    1684             isValid = false;
    16851679        break;
    16861680    case CalcPercentLength:
Note: See TracChangeset for help on using the changeset viewer.