Changeset 181352 in webkit


Ignore:
Timestamp:
Mar 10, 2015 4:19:38 PM (9 years ago)
Author:
Brent Fulgham
Message:

CSS scroll-snap-destination and scroll-snap-coordinate are not honoring position values
https://bugs.webkit.org/show_bug.cgi?id=142411

Reviewed by Simon Fraser.

Source/WebCore:

Tested by css3/scroll-snap/scroll-snap-position-values.html.

Revise the CSSParser to recognize that scroll-snap-coordinates and scroll-snap-destination
may be specified as positions, therefore allowing 'top', 'bottom', and 'center' for the Y axis,
and 'left', 'right', and 'center' for the X axis.

Correct implementation to support calculated values for Scroll Snap Point markup. This required the
Scroll Snap Point-specific LengthRepeat class to change its internal representation from a CSSPrimitiveValue
to a regular CSSValue.

Add tests that these position labels, as well as combinations with percentages and pixel offsets
are parsed properly.

  • css/CSSComputedStyleDeclaration.cpp:

(WebCore::scrollSnapDestination): Switch from 'percentageOrZoomAdjustedValue' to 'zoomAdjustedPixelValueForLength'
when working with Length values. This is necessary to allow calculated results to be based on the proper default
page dimensions.
(WebCore::scrollSnapPoints): Ditto.
(WebCore::scrollSnapCoordinates): Ditto.

  • css/CSSParser.cpp:

(WebCore::CSSParser::parseScrollSnapPositions): Consolidated code for dealing with snap point
positions.
(WebCore::CSSParser::parseScrollSnapDestination): Revise to call new helper function.
(WebCore::CSSParser::parseScrollSnapCoordinate): Ditto.
(WebCore::CSSParser::parseFillPositionX): Rename as parsePositionX.
(WebCore::CSSParser::parseFillPositionY): Rename as parsePositionY.
(WebCore::CSSParser::parseFillProperty): Update to call renamed parsePosition{X|Y} methods.
(WebCore::CSSParser::parseTransformOrigin): Ditto.
(WebCore::CSSParser::parsePerspectiveOrigin): Ditto.

  • css/CSSParser.h:
  • css/LengthRepeat.h: Revise class to use a CSSValue, rather than a CSSPrimitiveValue, so that we can represent

repeat values as calculations.

LayoutTests:

Add a test for <position> types in scroll snap operations. Also update the test expectations
for computed styles now that double-precision math is being used for calculated values.

  • css3/scroll-snap/scroll-snap-position-values-expected.txt: Added.
  • css3/scroll-snap/scroll-snap-position-values.html: Added.
  • css3/scroll-snap/scroll-snap-property-computed-style-expected.txt: Updated
  • css3/scroll-snap/scroll-snap-property-computed-style.js: Updated
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r181351 r181352  
     12015-03-10  Brent Fulgham  <bfulgham@apple.com>
     2
     3        CSS scroll-snap-destination and scroll-snap-coordinate are not honoring position values
     4        https://bugs.webkit.org/show_bug.cgi?id=142411
     5
     6        Reviewed by Simon Fraser.
     7
     8        Add a test for <position> types in scroll snap operations. Also update the test expectations
     9        for computed styles now that double-precision math is being used for calculated values.
     10
     11        * css3/scroll-snap/scroll-snap-position-values-expected.txt: Added.
     12        * css3/scroll-snap/scroll-snap-position-values.html: Added.
     13        * css3/scroll-snap/scroll-snap-property-computed-style-expected.txt: Updated
     14        * css3/scroll-snap/scroll-snap-property-computed-style.js: Updated
     15
     16
    1172015-03-10  Enrica Casucci  <enrica@apple.com>
    218
  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-property-computed-style-expected.txt

    r181189 r181352  
    156156
    157157mm along x axis with pixel repeat : 10mm repeat(42mm)
    158 PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-points-x') is '37.78125px repeat(158.734375px)'
     158PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-points-x') is '37.7952766418457px repeat(158.7401580810547px)'
    159159
    160160in along x axis with pixel repeat : 10in repeat(4in)
     
    162162
    163163pt along x axis with pixel repeat : 10pt repeat(42pt)
    164 PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-points-x') is '13.328125px repeat(56px)'
     164PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-points-x') is '13.333333015441895px repeat(56px)'
    165165
    166166in/cm destination : 2in 5cm
    167 PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-destination') is '192px 188.96875px'
     167PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-destination') is '192px 188.97637939453125px'
    168168
    169169in/cm coordinate : 2in 5cm 5in 2cm
    170 PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-coordinate') is '192px 188.96875px, 480px 75.578125px'
     170PASS window.getComputedStyle(document.body).getPropertyValue('-webkit-scroll-snap-coordinate') is '192px 188.97637939453125px, 480px 75.5905532836914px'
    171171
    172172subpixel along x axis with pixel repeat : 100.5px repeat(50.25px)
  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-property-computed-style.js

    r181189 r181352  
    7373testComputedScrollSnapRule("multiple percentage coordinates", "coordinate", "50% 100% 150% 100% 200% 100%", "50% 100%, 150% 100%, 200% 100%");
    7474
    75 testComputedScrollSnapRule("mm along x axis with pixel repeat", "points-x", "10mm repeat(42mm)", "37.78125px repeat(158.734375px)");
     75testComputedScrollSnapRule("mm along x axis with pixel repeat", "points-x", "10mm repeat(42mm)", "37.7952766418457px repeat(158.7401580810547px)");
    7676testComputedScrollSnapRule("in along x axis with pixel repeat", "points-x", "10in repeat(4in)", "960px repeat(384px)");
    77 testComputedScrollSnapRule("pt along x axis with pixel repeat", "points-x", "10pt repeat(42pt)", "13.328125px repeat(56px)");
    78 testComputedScrollSnapRule("in/cm destination", "destination", "2in 5cm", "192px 188.96875px");
    79 testComputedScrollSnapRule("in/cm coordinate", "coordinate", "2in 5cm 5in 2cm", "192px 188.96875px, 480px 75.578125px");
     77testComputedScrollSnapRule("pt along x axis with pixel repeat", "points-x", "10pt repeat(42pt)", "13.333333015441895px repeat(56px)");
     78testComputedScrollSnapRule("in/cm destination", "destination", "2in 5cm", "192px 188.97637939453125px");
     79testComputedScrollSnapRule("in/cm coordinate", "coordinate", "2in 5cm 5in 2cm", "192px 188.97637939453125px, 480px 75.5905532836914px");
    8080
    8181testComputedScrollSnapRule("subpixel along x axis with pixel repeat", "points-x", "100.5px repeat(50.25px)", "100.5px repeat(50.25px)");
  • trunk/Source/WebCore/ChangeLog

    r181351 r181352  
     12015-03-10  Brent Fulgham  <bfulgham@apple.com>
     2
     3        CSS scroll-snap-destination and scroll-snap-coordinate are not honoring position values
     4        https://bugs.webkit.org/show_bug.cgi?id=142411
     5
     6        Reviewed by Simon Fraser.
     7
     8        Tested by css3/scroll-snap/scroll-snap-position-values.html.
     9
     10        Revise the CSSParser to recognize that scroll-snap-coordinates and scroll-snap-destination
     11        may be specified as positions, therefore allowing 'top', 'bottom', and 'center' for the Y axis,
     12        and 'left', 'right', and 'center' for the X axis.
     13
     14        Correct implementation to support calculated values for Scroll Snap Point markup. This required the
     15        Scroll Snap Point-specific LengthRepeat class to change its internal representation from a CSSPrimitiveValue
     16        to a regular CSSValue.
     17
     18        Add tests that these position labels, as well as combinations with percentages and pixel offsets
     19        are parsed properly.
     20
     21        * css/CSSComputedStyleDeclaration.cpp:
     22        (WebCore::scrollSnapDestination): Switch from 'percentageOrZoomAdjustedValue' to 'zoomAdjustedPixelValueForLength'
     23        when working with Length values. This is necessary to allow calculated results to be based on the proper default
     24        page dimensions.
     25        (WebCore::scrollSnapPoints): Ditto.
     26        (WebCore::scrollSnapCoordinates): Ditto.
     27        * css/CSSParser.cpp:
     28        (WebCore::CSSParser::parseScrollSnapPositions): Consolidated code for dealing with snap point
     29        positions.
     30        (WebCore::CSSParser::parseScrollSnapDestination): Revise to call new helper function.
     31        (WebCore::CSSParser::parseScrollSnapCoordinate): Ditto.
     32        (WebCore::CSSParser::parseFillPositionX): Rename as parsePositionX.
     33        (WebCore::CSSParser::parseFillPositionY): Rename as parsePositionY.
     34        (WebCore::CSSParser::parseFillProperty): Update to call renamed parsePosition{X|Y} methods.
     35        (WebCore::CSSParser::parseTransformOrigin): Ditto.
     36        (WebCore::CSSParser::parsePerspectiveOrigin): Ditto.
     37        * css/CSSParser.h:
     38        * css/LengthRepeat.h: Revise class to use a CSSValue, rather than a CSSPrimitiveValue, so that we can represent
     39        repeat values as calculations.
     40
    1412015-03-10  Enrica Casucci  <enrica@apple.com>
    242
  • trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

    r181189 r181352  
    11101110{
    11111111    auto list = CSSValueList::createSpaceSeparated();
    1112     list.get().append(percentageOrZoomAdjustedValue(destination.width(), &style));
    1113     list.get().append(percentageOrZoomAdjustedValue(destination.height(), &style));
     1112    list.get().append(zoomAdjustedPixelValueForLength(destination.width(), &style));
     1113    list.get().append(zoomAdjustedPixelValueForLength(destination.height(), &style));
    11141114    return list;
    11151115}
     
    11241124    auto list = CSSValueList::createSpaceSeparated();
    11251125    for (auto& point : points->offsets)
    1126         list.get().append(percentageOrZoomAdjustedValue(point, &style));
     1126        list.get().append(zoomAdjustedPixelValueForLength(point, &style));
    11271127    if (points->hasRepeat)
    1128         list.get().append(cssValuePool().createValue(LengthRepeat::create(percentageOrZoomAdjustedValue(points->repeatOffset, &style))));
     1128        list.get().append(cssValuePool().createValue(LengthRepeat::create(zoomAdjustedPixelValueForLength(points->repeatOffset, &style))));
    11291129    return WTF::move(list);
    11301130}
     
    11391139    for (auto& coordinate : coordinates) {
    11401140        auto pair = CSSValueList::createSpaceSeparated();
    1141         pair.get().append(percentageOrZoomAdjustedValue(coordinate.width(), &style));
    1142         pair.get().append(percentageOrZoomAdjustedValue(coordinate.height(), &style));
     1141        pair.get().append(zoomAdjustedPixelValueForLength(coordinate.width(), &style));
     1142        pair.get().append(zoomAdjustedPixelValueForLength(coordinate.height(), &style));
    11431143        list.get().append(WTF::move(pair));
    11441144    }
  • trunk/Source/WebCore/css/CSSParser.cpp

    r181197 r181352  
    33203320}
    33213321
     3322bool CSSParser::parseScrollSnapPositions(RefPtr<CSSValue>& cssValueX, RefPtr<CSSValue>& cssValueY)
     3323{
     3324    cssValueX = parsePositionX(*m_valueList);
     3325    if (!cssValueX)
     3326        return false;
     3327
     3328    // Don't accept odd-length lists of positions (must always have an X and a Y):
     3329    if (!m_valueList->next())
     3330        return false;
     3331
     3332    cssValueY = parsePositionY(*m_valueList);
     3333    if (!cssValueY)
     3334        return false;
     3335
     3336    return true;
     3337}
     3338
    33223339bool CSSParser::parseScrollSnapDestination(CSSPropertyID propId, bool important)
    33233340{
     
    33253342    if (m_valueList->size() != 2)
    33263343        return false;
    3327     ValueWithCalculation valueXWithCalculation(*m_valueList->current());
    3328     if (!validateUnit(valueXWithCalculation, FPercent | FLength))
    3329         return false;
    3330     RefPtr<CSSValue> cssValueX = createPrimitiveNumericValue(valueXWithCalculation);
    3331 
    3332     ValueWithCalculation valueYWithCalculation(*m_valueList->next());
    3333     if (!validateUnit(valueYWithCalculation, FPercent | FLength))
    3334         return false;
    3335     RefPtr<CSSValue> cssValueY = createPrimitiveNumericValue(valueYWithCalculation);
     3344
     3345    RefPtr<CSSValue> cssValueX, cssValueY;
     3346    if (!parseScrollSnapPositions(cssValueX, cssValueY))
     3347        return false;
     3348
    33363349    position->append(cssValueX.releaseNonNull());
    33373350    position->append(cssValueY.releaseNonNull());
     
    33453358    RefPtr<CSSValueList> positions = CSSValueList::createSpaceSeparated();
    33463359    while (m_valueList->current()) {
    3347         ValueWithCalculation valueXWithCalculation(*m_valueList->current());
    3348         // Don't accept odd-length lists of coordinates.
    3349         if (!m_valueList->next())
     3360        RefPtr<CSSValue> cssValueX, cssValueY;
     3361        if (!parseScrollSnapPositions(cssValueX, cssValueY))
    33503362            return false;
    3351         ValueWithCalculation valueYWithCalculation(*m_valueList->current());
    3352         if (!validateUnit(valueXWithCalculation, FPercent | FLength) || !validateUnit(valueYWithCalculation, FPercent | FLength))
    3353             return false;
    3354         positions->append(createPrimitiveNumericValue(valueXWithCalculation));
    3355         positions->append(createPrimitiveNumericValue(valueYWithCalculation));
     3363
     3364        positions->append(cssValueX.releaseNonNull());
     3365        positions->append(cssValueY.releaseNonNull());
    33563366        m_valueList->next();
    33573367    }
     3368
    33583369    if (positions->length()) {
    33593370        addProperty(propId, positions.release(), important);
     
    40384049}
    40394050
    4040 PassRefPtr<CSSValue> CSSParser::parseFillPositionX(CSSParserValueList& valueList)
     4051PassRefPtr<CSSValue> CSSParser::parsePositionX(CSSParserValueList& valueList)
    40414052{
    40424053    int id = valueList.current()->id;
     
    40554066}
    40564067
    4057 PassRefPtr<CSSValue> CSSParser::parseFillPositionY(CSSParserValueList& valueList)
     4068PassRefPtr<CSSValue> CSSParser::parsePositionY(CSSParserValueList& valueList)
    40584069{
    40594070    int id = valueList.current()->id;
     
    45654576                case CSSPropertyBackgroundPositionX:
    45664577                case CSSPropertyWebkitMaskPositionX: {
    4567                     currValue = parseFillPositionX(*m_valueList);
     4578                    currValue = parsePositionX(*m_valueList);
    45684579                    if (currValue)
    45694580                        m_valueList->next();
     
    45724583                case CSSPropertyBackgroundPositionY:
    45734584                case CSSPropertyWebkitMaskPositionY: {
    4574                     currValue = parseFillPositionY(*m_valueList);
     4585                    currValue = parsePositionY(*m_valueList);
    45754586                    if (currValue)
    45764587                        m_valueList->next();
     
    97159726            break;
    97169727        case CSSPropertyWebkitTransformOriginX: {
    9717             value = parseFillPositionX(*m_valueList);
     9728            value = parsePositionX(*m_valueList);
    97189729            if (value)
    97199730                m_valueList->next();
     
    97219732        }
    97229733        case CSSPropertyWebkitTransformOriginY: {
    9723             value = parseFillPositionY(*m_valueList);
     9734            value = parsePositionY(*m_valueList);
    97249735            if (value)
    97259736                m_valueList->next();
     
    97589769            break;
    97599770        case CSSPropertyWebkitPerspectiveOriginX: {
    9760             value = parseFillPositionX(*m_valueList);
     9771            value = parsePositionX(*m_valueList);
    97619772            if (value)
    97629773                m_valueList->next();
     
    97649775        }
    97659776        case CSSPropertyWebkitPerspectiveOriginY: {
    9766             value = parseFillPositionY(*m_valueList);
     9777            value = parsePositionY(*m_valueList);
    97679778            if (value)
    97689779                m_valueList->next();
  • trunk/Source/WebCore/css/CSSParser.h

    r181209 r181352  
    155155    enum FillPositionParsingMode { ResolveValuesAsPercent = 0, ResolveValuesAsKeyword = 1 };
    156156    PassRefPtr<CSSPrimitiveValue> parseFillPositionComponent(CSSParserValueList&, unsigned& cumulativeFlags, FillPositionFlag& individualFlag, FillPositionParsingMode = ResolveValuesAsPercent);
    157     PassRefPtr<CSSValue> parseFillPositionX(CSSParserValueList&);
    158     PassRefPtr<CSSValue> parseFillPositionY(CSSParserValueList&);
     157    PassRefPtr<CSSValue> parsePositionX(CSSParserValueList&);
     158    PassRefPtr<CSSValue> parsePositionY(CSSParserValueList&);
    159159    void parse2ValuesFillPosition(CSSParserValueList&, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
    160160    bool isPotentialPositionValue(CSSParserValue&);
     
    560560    bool parseScrollSnapDestination(CSSPropertyID propId, bool important);
    561561    bool parseScrollSnapCoordinate(CSSPropertyID propId, bool important);
     562    bool parseScrollSnapPositions(RefPtr<CSSValue>& cssValueX, RefPtr<CSSValue>& cssValueY);
    562563#endif
    563564
  • trunk/Source/WebCore/css/LengthRepeat.h

    r172192 r181352  
    11/*
    2  * Copyright (C) 2014 Apple Inc. All rights reserved.
     2 * Copyright (C) 2014-2015 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    3737class LengthRepeat : public RefCounted<LengthRepeat> {
    3838public:
    39     static PassRefPtr<LengthRepeat> create(PassRefPtr<CSSPrimitiveValue> interval) { return adoptRef(new LengthRepeat(interval)); }
     39    static PassRefPtr<LengthRepeat> create(PassRefPtr<CSSValue> interval) { return adoptRef(new LengthRepeat(interval)); }
    4040
    4141    PassRefPtr<LengthRepeat> cloneForCSSOM() const { return create(interval()); }
    4242
    43     CSSPrimitiveValue* interval() const { return m_interval.get(); }
     43    CSSValue* interval() const { return m_interval.get(); }
    4444
    45     void setInterval(PassRefPtr<CSSPrimitiveValue> interval) { m_interval = interval; }
     45    void setInterval(PassRefPtr<CSSValue> interval) { m_interval = interval; }
    4646
    4747    bool equals(const LengthRepeat& other) const
     
    5656
    5757private:
    58     LengthRepeat(PassRefPtr<CSSPrimitiveValue> interval)
     58    LengthRepeat(PassRefPtr<CSSValue> interval)
    5959        : m_interval(interval)
    6060    {
    6161    }
    6262
    63     RefPtr<CSSPrimitiveValue> m_interval;
     63    RefPtr<CSSValue> m_interval;
    6464};
    6565
  • trunk/Source/WebCore/css/StyleBuilderConverter.h

    r181189 r181352  
    706706inline Length StyleBuilderConverter::parseSnapCoordinate(StyleResolver& styleResolver, const CSSValue& value)
    707707{
    708     return downcast<CSSPrimitiveValue>(value).convertToLength<FixedIntegerConversion | PercentConversion | AutoConversion>(styleResolver.state().cssToLengthConversionData());
     708    return downcast<CSSPrimitiveValue>(value).convertToLength<FixedIntegerConversion | PercentConversion | CalculatedConversion | AutoConversion>(styleResolver.state().cssToLengthConversionData());
    709709}
    710710
Note: See TracChangeset for help on using the changeset viewer.