Bug 160934

Summary: Support image-set() standard syntax
Product: WebKit Reporter: yisibl <50167214>
Component: CSSAssignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Normal CC: 50167214, bfulgham, commit-queue, dino, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jonlee, macpherson, menard, m.goleb+bugzilla, nicolas, noam, simon.fraser, sunil.ratnu, webkit-bug-importer
Priority: P2 Keywords: BrowserCompat, InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
URL: https://drafts.csswg.org/css-images/#image-set-notation
See Also: https://bugs.webkit.org/show_bug.cgi?id=159373
https://bugs.webkit.org/show_bug.cgi?id=206909
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description yisibl 2016-08-16 21:18:51 PDT
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
Comment 1 Radar WebKit Bug Importer 2016-08-17 12:41:09 PDT
<rdar://problem/27891501>
Comment 2 Sunil Ratnu 2016-09-22 06:51:52 PDT
Is WebKit planning to support the above standard syntax in near future?
Comment 3 Dean Jackson 2016-09-22 08:54:28 PDT
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?
Comment 4 Sunil Ratnu 2016-09-22 23:09:53 PDT
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?
Comment 5 yisibl 2016-09-23 03:01:21 PDT
Yeah Sunil Ratnu is right
Comment 6 Noam Rosenthal 2020-01-09 13:38:30 PST
Created attachment 387268 [details]
Patch
Comment 7 Simon Fraser (smfr) 2020-01-09 14:08:19 PST
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?
Comment 8 Noam Rosenthal 2020-01-09 23:08:10 PST
 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
Comment 9 Noam Rosenthal 2020-01-09 23:16:56 PST
Created attachment 387315 [details]
Patch
Comment 10 Noam Rosenthal 2020-01-09 23:19:40 PST
Created attachment 387316 [details]
Patch
Comment 11 Noam Rosenthal 2020-01-10 00:48:16 PST
Created attachment 387318 [details]
Patch
Comment 12 Noam Rosenthal 2020-01-10 00:49:01 PST
Had to update some tests, would like a re-review of them (once EWS succeeds).
Thanks!
Comment 13 Noam Rosenthal 2020-01-10 01:34:58 PST
Created attachment 387319 [details]
Patch
Comment 14 Dean Jackson 2020-01-11 12:43:07 PST
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
> +2020-01-09  Noam Rosenthal  <noam@webkit.org>
> +
> +        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.
Comment 15 WebKit Commit Bot 2020-01-11 16:29:42 PST
Comment on attachment 387319 [details]
Patch

Clearing flags on attachment: 387319

Committed r254406: <https://trac.webkit.org/changeset/254406>
Comment 16 WebKit Commit Bot 2020-01-11 16:29:44 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 Noam Rosenthal 2020-01-12 02:05:41 PST
Added bug report for dpi:
https://bugs.webkit.org/show_bug.cgi?id=206142
Comment 18 Simon Fraser (smfr) 2020-01-28 14:45:58 PST
This broke gmail: bug 206909