Bug 160934 - Support image-set() standard syntax
Summary: Support image-set() standard syntax
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Noam Rosenthal
URL: https://drafts.csswg.org/css-images/#...
Keywords: BrowserCompat, InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2016-08-16 21:18 PDT by yisibl
Modified: 2020-01-28 14:47 PST (History)
16 users (show)

See Also:


Attachments
Patch (6.80 KB, patch)
2020-01-09 13:38 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (6.92 KB, patch)
2020-01-09 23:16 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (6.92 KB, patch)
2020-01-09 23:19 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (9.38 KB, patch)
2020-01-10 00:48 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (11.66 KB, patch)
2020-01-10 01:34 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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