Support image-set() standard syntax

[yisibl]

In spec[1] says:

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 );


[1] https://drafts.csswg.org/css-images/#image-set-notation

[Radar WebKit Bug Importer]

<rdar://problem/27891501>

[Sunil Ratnu]

Is WebKit planning to support the above standard syntax in near future?

[Dean Jackson]

I'm not sure what part of the syntax this bug is requesting. We don't yet support the dpi option, but it doesn't sound like this is the complaint.

Can you provide a test case that shows where we are failing?

[Sunil Ratnu]

Currently Blink supports this syntax:

[1]
background-image: image-set( url("foo.png") 1x,
                             url("foo-2x.png") 2x,
                             url("foo-print.png") 3x );


From standard syntax, I mean the following:
[2]
background-image: image-set( "foo.png" 1x,
                             url("foo-2x.png") 2x,
                             "foo-print.png" 600dpi );

Looking at this, it can be seen that the standard syntax (i.e. [2]) is superset of currently supported syntax (i.e. [1]).

So, basically, you do not need url("..."), you can enter it as a string without the url() function. Also, the resolution part supports more than just #x (i.e. it supports #x as well #dpi)

Does WebKit have plans to support this standard syntax?

[yisibl]

Yeah Sunil Ratnu is right

[Noam Rosenthal]

Created attachment 387268 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 387268 [details]
Patch

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

> LayoutTests/fast/hidpi/image-set-as-background-quotes-expected.html:18
> +            // Right now there is a bug that image-set does not properly deal with dynamic changes to the scale factor,

Link to the bug?

[Noam Rosenthal]

 LayoutTests/fast/hidpi/image-set-as-background-quotes-expected.html:18
> > +            // Right now there is a bug that image-set does not properly deal with dynamic changes to the scale factor,


> Link to the bug?

I'm not sure! I'll look it up next, this file is a copy of LayoutTests/fast/hidpi/image-set-as-background.html

[Noam Rosenthal]

Created attachment 387315 [details]
Patch

[Noam Rosenthal]

Created attachment 387316 [details]
Patch

[Noam Rosenthal]

Created attachment 387318 [details]
Patch

[Noam Rosenthal]

Had to update some tests, would like a re-review of them (once EWS succeeds).
Thanks!

[Noam Rosenthal]

Created attachment 387319 [details]
Patch

[Dean Jackson]

Comment on attachment 387319 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Support image-set() standard syntax

I think this bug should be named Support images URLs without the url() syntax in image-set.

And maybe we should open another bug for the missing dpi identifier?

> LayoutTests/ChangeLog:28
> [phone]  Noam Rosenthal  <[e-mail]>
> +
> +        Support image-set() standard syntax
> +        https://bugs.webkit.org/show_bug.cgi?id=160934
> +        <rdar://problem/27891501>
> +
> +        Reviewed by Simon Fraser.
> +
> +
> +        * fast/hidpi/image-set-as-background-quotes-expected.html: Added.
> +        * fast/hidpi/image-set-as-background-quotes.html: Added.
> +                Ref-test, image with quotes should work the same as image with url()

Ooops. Duplicated log entry.

[WebKit Commit Bot]

Comment on attachment 387319 [details]
Patch

Clearing flags on attachment: 387319

Committed r254406: <https://trac.webkit.org/changeset/254406>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Noam Rosenthal]

Added bug report for dpi:
https://bugs.webkit.org/show_bug.cgi?id=206142

[Simon Fraser (smfr)]

This broke gmail: bug 206909