Bug 159373 – "image-src" support is missing. We only support "-webkit-image-src"

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Brent Fulgham]

We only support the prefixed version of "image-src" (i.e., "-webkit-image-src").

We should support the unprefixed variant of this CSS.

[Brent Fulgham]

Created attachment 282590 [details]
Test case showing the null image

Test case showing "image-src(...)" is not supported

[Radar WebKit Bug Importer]

<rdar://problem/27140443>

[Brent Fulgham]

Created attachment 282608 [details]
Patch

[Dean Jackson]

Comment on attachment 282608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282608&action=review

> Source/WebCore/css/CSSParser.cpp:2096
> +            } else if (value->unit == CSSParserValue::Function && (equalLettersIgnoringASCIICase(value->function->name, "image-set(")
> +                || equalLettersIgnoringASCIICase(value->function->name, "-webkit-image-set("))) {

I did this:

+static bool isImageSetFunctionValue(const CSSParserValue& value)
+{
+    return value.unit == CSSParserValue::Function && (equalLettersIgnoringASCIICase(value.function->name, "image-set(") || equalLettersIgnoringASCIICase(value.function->name, "-webkit-image-set("));
+}
+

And then used it throughout CSSParser.cpp

> LayoutTests/fast/css/script-tests/image-set-setting.js:18
> -    var fullRule = "-webkit-image-set(" + rule + ")";
> +    var fullRule = "image-set(" + rule + ")";

I think we should have this test both forms.

[Dean Jackson]

Created attachment 282609 [details]
CSSParser.diff

[Brent Fulgham]

Comment on attachment 282608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282608&action=review

>> Source/WebCore/css/CSSParser.cpp:2096
>> +                || equalLettersIgnoringASCIICase(value->function->name, "-webkit-image-set("))) {
> 
> I did this:
> 
> +static bool isImageSetFunctionValue(const CSSParserValue& value)
> +{
> +    return value.unit == CSSParserValue::Function && (equalLettersIgnoringASCIICase(value.function->name, "image-set(") || equalLettersIgnoringASCIICase(value.function->name, "-webkit-image-set("));
> +}
> +
> 
> And then used it throughout CSSParser.cpp

That's much better.

>> LayoutTests/fast/css/script-tests/image-set-setting.js:18
>> +    var fullRule = "image-set(" + rule + ")";
> 
> I think we should have this test both forms.

I tried that initially, but the behavior of 'cssText' causes trouble.

If I create a rule using "-webkit-image-set" it round-trips as "image-set", which breaks the test.

[Dean Jackson]

Committed r202765: <http://trac.webkit.org/changeset/202765>

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> Committed r202765: <http://trac.webkit.org/changeset/202765>

Layout test results fix for iOS Simulator:

Committed r202786: <https://trac.webkit.org/changeset/202786>

[yisibl]

Please support the standard syntax: 

Each <string> inside image-set() represents a <url>, just like in image().

image-set( "foo.png" 1x, "foo-2x.png" 2x, "foo-print.png" 600dpi );

[Brent Fulgham]

(In reply to comment #9)
> Please support the standard syntax: 
> 
> Each <string> inside image-set() represents a <url>, just like in image().
> 
> image-set( "foo.png" 1x, "foo-2x.png" 2x, "foo-print.png" 600dpi );

Could you please file a new Bug for this issue so we can track it more easily, since this bug is already closed?

Thanks!