Changeset 255228 in webkit for trunk/Source/WebCore/css/CSSImageSetValue.cpp

Timestamp:

Jan 27, 2020, 11:54:48 PM (5 years ago)

Author:

Noam Rosenthal

Message:

-webkit-image-set should support resolution units other than 'x'
​https://bugs.webkit.org/show_bug.cgi?id=100120

Reviewed by Darin Adler.

Source/WebCore:

Leveraged previous work on image-resolution to enable dpi/dppx/dpcm in CSS image-set.
Now the second value of every entry in the image-set is parsed like any resolution,
allowing "x" as a synonim for "dppx.

This changes computed style behavior for image-set - "x" resolution values will be converted
to "dppx". This is in line with the spirit of computed values, though it's not spec'ed particularly.

Tests: fast/hidpi/image-set-units.html

• css/CSSImageSetValue.cpp:

(WebCore::CSSImageSetValue::fillImageSet):
(WebCore::CSSImageSetValue::updateDeviceScaleFactor):
(WebCore::CSSImageSetValue::customCSSText const):

Convert values to dppx before sorting them the image set.
Use given CSS units in computed styles instead of hardcoding "x"

• css/CSSPrimitiveValue.cpp:

(WebCore::isValidCSSUnitTypeForDoubleConversion):
(WebCore::CSSPrimitiveValue::conversionToCanonicalUnitsScaleFactor):

Allow conversion of dpi/dppx/dpcm to canonical (dppx).

• css/parser/CSSPropertyParserHelpers.cpp:

(WebCore::CSSPropertyParserHelpers::consumeResolution):
(WebCore::CSSPropertyParserHelpers::consumeImageSet):

• css/parser/CSSPropertyParserHelpers.h:

Consume any resolution when parsing image-set.

LayoutTests:

Had to modify several image-set parsing tests because of the behavior change in
computed style (x => dppx).

Also added a couple of parsing/rendering tests specifically to test the new behavior
of dpi/dpcm/dppx.

• fast/css/cursor-parsing-image-set-expected.txt:
• fast/css/cursor-parsing-image-set.html:
• fast/css/image-set-parsing-expected.txt:
• fast/css/image-set-parsing-invalid-expected.txt:
• fast/css/image-set-parsing.html:
• fast/css/image-set-setting-expected.txt:
• fast/css/image-set-setting.html:
• fast/css/image-set-unprefixed.html:
• fast/hidpi/image-set-units-expected.html: Added.
• fast/hidpi/image-set-units.html: Added.

File:

• trunk/Source/WebCore/css/CSSImageSetValue.cpp (modified) (3 diffs)

trunk/Source/WebCore/css/CSSImageSetValue.cpp

@@ -51 +51 @@
 {
     size_t length = this->length();
-    size_t i = 0;
-    while (i < length) {
+    for (size_t i = 0; i + 1 < length; i += 2) {
         CSSValue* imageValue = item(i);
+        CSSValue* scaleFactorValue = item(i + 1);
+
         ASSERT(is<CSSImageValue>(imageValue) || is<CSSImageGeneratorValue>(imageValue));
-        ++i;
-        ASSERT_WITH_SECURITY_IMPLICATION(i < length);
-        CSSValue* scaleFactorValue = item(i);
-        float scaleFactor = downcast<CSSPrimitiveValue>(*scaleFactorValue).floatValue();
+        ASSERT(is<CSSPrimitiveValue>(scaleFactorValue));
 
-        ImageWithScale image;
-        image.value = imageValue;
-        image.scaleFactor = scaleFactor;
-        m_imagesInSet.append(image);
-        ++i;
+        float scaleFactor = downcast<CSSPrimitiveValue>(scaleFactorValue)->floatValue(CSSUnitType::CSS_DPPX);
+        m_imagesInSet.append({ imageValue, scaleFactor });
     }
 
@@ -107 +102 @@
 void CSSImageSetValue::updateDeviceScaleFactor(const Document& document)
 {
-
     // FIXME: In the future, we want to take much more than deviceScaleFactor into acount here.
     // All forms of scale should be included: Page::pageScaleFactor(), Frame::pageZoomFactor(),
@@ -137 +131 @@
 
     size_t length = this->length();
-    size_t i = 0;
-    while (i < length) {
+    for (size_t i = 0; i + 1 < length; i += 2) {
         if (i > 0)
             result.appendLiteral(", ");
 
-        const CSSValue* imageValue = item(i);
-        result.append(imageValue->cssText());
-        result.append(' ');
-
-        ++i;
-        ASSERT_WITH_SECURITY_IMPLICATION(i < length);
-        const CSSValue* scaleFactorValue = item(i);
-        result.append(scaleFactorValue->cssText());
-        // FIXME: Eventually the scale factor should contain it's own unit http://wkb.ug/100120.
-        // For now 'x' is hard-coded in the parser, so we hard-code it here too.
-        result.append('x');
-
-        ++i;
+        result.append(item(i)->cssText(), ' ', item(i + 1)->cssText());
     }