[3003] Allow mathematical expressions in image sizes #3084

musgravejw · 2017-09-30T06:48:10Z

This PR attempts to clarify that mathematical functions "min()" and "max()" can also be used as a valid arguments for source-size-value in Image size attributes.

#3003

https://drafts.csswg.org/css-values/#math-functions

/images.html ( diff )
/infrastructure.html ( diff )

annevk · 2017-09-30T15:31:28Z

Thank you! We'll also need to update https://html.spec.whatwg.org/#parsing-a-sizes-attribute. (I wonder how that algorithm works, when is the value computed? It seems that with calc() you could easily end up with a negative value, but not know it yet.)

And we'll need additional test coverage over at https://github.com/w3c/web-platform-tests.

musgravejw · 2017-10-02T16:39:34Z

@annevk Ah, you're right, I missed that reference to calc(). I will update the web platform tests as well. What would be the best way to document the algorithm for calc() returning negative values? Also, could you point me in the right direction for documentation? Looks like the CSSWG covers parsing.

annevk · 2017-10-02T17:23:02Z

Thank you!

I hope @zcorpan or @tabatkins can help out with the parsing question. I don't really know how this is supposed to work unfortunately.

zcorpan · 2017-10-04T07:59:59Z

The entry points to parsing sizes are from https://html.spec.whatwg.org/multipage/images.html#update-the-source-set

which is invoked from https://html.spec.whatwg.org/multipage/images.html#select-an-image-source

which is invoked from
https://html.spec.whatwg.org/multipage/images.html#updating-the-image-data:select-an-image-source
and
https://html.spec.whatwg.org/multipage/images.html#reacting-to-environment-changes:select-an-image-source

The calc() value is evaluated during parsing, so you should know what it evaluates to right away.

A source size is a <source-size-value>. When a source size has a unit relative to the viewport, it must be interpreted relative to the img element's node document's viewport. Other units must be interpreted the same as in Media Queries. [MQ]

https://html.spec.whatwg.org/multipage/images.html#source-size-2

("the same as in Media Queries" means that font-relative units use initial values, so 1em is the user's default font size, not the img element's computed 'font-size'.)

However, a mathematical function could evaluate differently each time sizes is parsed, and indeed it's possible it could be negative some of the time (e.g. for calc(500px - 50vw)). I don't remember why we made it invalid (and skipped over in parsing), rather than clamping to 0. The latter seems like it's less surprising behavior, so I'm open to changing that, but that's probably a separate issue and doesn't need to block this PR imo.

zcorpan · 2017-10-04T07:36:57Z

source

@@ -26775,7 +26777,7 @@ was an English &lt;a href="/wiki/Music_hall">music hall&lt;/a> singer, ...</pre>
 <dfn>&lt;source-size-value></dfn> = <span>&lt;length></span></pre>

  <p>A <span>&lt;source-size-value></span> must not be negative, and must not use CSS functions
-  other than the <span>'calc()'</span> function.</p>
+  other than the mathematical functions <span>'calc()'</span>, <span>'min()'</span>, and <span>'max()'</span>.</p>


I'd prefer to cross-reference "mathematical functions" and not limit to these 3 -- if CSS adds more mathematical functions, they should work here as well.

other than the mathematical functions (such as 'calc()', 'min()', and 'max()').

zcorpan · 2017-10-04T07:38:22Z

source

@@ -27929,7 +27931,7 @@ was an English &lt;a href="/wiki/Music_hall">music hall&lt;/a> singer, ...</pre>
   is a valid non-negative <span>&lt;source-size-value></span>,
   let <var>size</var> be its value
   and remove the <span>component value</span> from <var>unparsed size</var>.
-   Any CSS function other than the <span>'calc()'</span> function is invalid.
+   Any CSS function other than the mathematical functions <span>'calc()'</span>, <span>'min()'</span>, and <span>'max()'</span> is invalid.


Just cross-reference mathematical functions here without listing what they are.

zcorpan · 2017-10-04T08:05:57Z

The relevant test is /html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute.html

annevk · 2017-10-09T07:20:43Z

Mathematical functions should probably not have single quotes around it?

zcorpan · 2017-10-09T10:07:57Z

@annevk indeed.

annevk · 2017-10-09T10:21:44Z

source

@@ -3700,7 +3700,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
     <li>The <dfn data-x-href="https://drafts.csswg.org/css-values/#in">'in'</dfn> unit</li>
     <li>The <dfn data-x-href="https://drafts.csswg.org/css-values/#px">'px'</dfn> unit</li>
     <li>The <dfn data-x-href="https://drafts.csswg.org/css-values/#funcdef-attr">'attr()'</dfn> function</li>
-     <li>The <dfn data-x-href="https://drafts.csswg.org/css-values/#funcdef-calc">'calc()'</dfn> function</li>
+     <li>The <dfn data-x-href="https://drafts.csswg.org/css-values/#calc-notation">'mathematical functions'</dfn></li>


We should call this "math functions" and link to https://drafts.csswg.org/css-values/#math-functions.

musgravejw · 2017-10-11T01:16:47Z

Thanks @sideshowbarker... ugh, so many dumb mistakes on my part:

https://media.giphy.com/media/bkklBjAmlYjv2/giphy.gif

musgravejw · 2017-10-11T01:17:35Z

@zcorpan I had a question about the W3C test, should I open an issue there instead?

annevk · 2017-10-11T03:33:59Z

@musgravejw up to you. You should feel free to ask questions here about tests as well. Note that per https://whatwg.org/working-mode#changes we'll wait for the tests before merging.

musgravejw · 2017-10-17T01:05:01Z

Test suite is throwing an exception for me, I have opened an issue in the web-platform-tests repo, and will update as soon as I can get it resolved.

donovanglover · 2018-03-02T00:15:52Z

Hi @musgravejw,

Mind if I try working on the tests?

musgravejw · 2018-03-02T00:38:01Z

@gloverdonovan Not at all, that would be really helpful actually. Sorry I didn't mean to leave this hanging, I have been meaning to wrap it up as soon as I had some bandwidth.

Thank you.

domfarolino · 2018-03-23T19:48:30Z

How's it going @gloverdonovan ?

donovanglover · 2018-03-24T06:42:21Z

Honestly forgot about this issue. I don't think I can dedicate the time to figure out how to write the tests. If anyone else can do it that'd be great.

Modify attribute parsing

domfarolino · 2018-03-24T16:27:19Z

I rebased this PR (because I couldn't build without linting errors in the current state) and fixed the CSSWG link (the link seems to be singular and not plural).

For right now, seems like the next step is to have @musgravejw sign the participant agreement (https://participate.whatwg.org/agreement) as an individual.

I'll take a look at some tests for this.

domfarolino · 2018-03-25T05:16:55Z

I've added web-platform-tests/wpt#10167 which should suffice for testing this change.

annevk · 2018-03-26T07:50:37Z

Great, thanks all! So the one thing still lacking is browser bugs. https://github.com/whatwg/meta/blob/master/MAINTAINERS.md#handling-pull-requests has links for each browser's bug database if someone would like to take that on. Please paste the resulting bugs URLs here.

domfarolino · 2018-03-26T17:19:03Z

I've filed https://crbug.com/825895 for Chromium, @musgravejw how do you feel about taking some of the others? That Chromium issue should be a good template to follow :)

musgravejw · 2018-03-27T01:52:22Z

Remaining bugs filed here:

https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/16587544/
https://bugzilla.mozilla.org/show_bug.cgi?id=1449070
https://bugs.webkit.org/show_bug.cgi?id=184027

Tests for whatwg/html#3084.

annevk · 2018-04-09T12:10:02Z

Thanks again all, the change is now official! 🎂

If anyone is interested in more, #3093 might be a nice follow-up to fix.

…<img sizes="">, a=testonly Automatic update from web-platform-testsAdd tests for mathematical functions in <img sizes=""> Tests for whatwg/html#3084. wpt-commits: cc9648dde51ca42dfa40d1ff05c017dd3966929f wpt-pr: 10167 wpt-commits: cc9648dde51ca42dfa40d1ff05c017dd3966929f wpt-pr: 10167

…<img sizes="">, a=testonly Automatic update from web-platform-testsAdd tests for mathematical functions in <img sizes=""> Tests for whatwg/html#3084. wpt-commits: cc9648dde51ca42dfa40d1ff05c017dd3966929f wpt-pr: 10167 wpt-commits: cc9648dde51ca42dfa40d1ff05c017dd3966929f wpt-pr: 10167 UltraBlame original commit: 38df35b53e6b8c3fb67629a873c98401edb1492f

annevk added topic: img needs tests Moving the issue forward requires someone to write tests labels Sep 30, 2017

zcorpan reviewed Oct 4, 2017

View reviewed changes

zcorpan mentioned this pull request Oct 4, 2017

Consider allowing negative source sizes in <img sizes>, clamping to 0 #3093

Open

annevk reviewed Oct 9, 2017

View reviewed changes

annevk approved these changes Oct 11, 2017

View reviewed changes

annevk mentioned this pull request Oct 22, 2017

wpt run for Chrome throws Python Attribute Error exception web-platform-tests/wpt#7854

Closed

This was referenced Feb 19, 2018

Allow min() and max() in <img sizes=""> #3003

Closed

Allow min() and max() for the image sizes attribute #3476

Closed

musgravejw and others added 2 commits March 24, 2018 12:18

[#3003] Allow mathematical expressions in image sizes

d3810fe

Modify attribute parsing

Update css-values link

c4e2f17

domfarolino added a commit to domfarolino/web-platform-tests that referenced this pull request Mar 25, 2018

Add tests for whatwg/html#3084

bff298e

domfarolino mentioned this pull request Mar 25, 2018

Add tests for mathematical functions in <img sizes=""> web-platform-tests/wpt#10167

Merged

annevk pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 9, 2018

Add tests for mathematical functions in <img sizes="">

cc9648d

Tests for whatwg/html#3084.

annevk merged commit 5d091c2 into whatwg:master Apr 9, 2018

domfarolino deleted the 3003-img-size branch April 9, 2018 16:55

domfarolino mentioned this pull request Apr 10, 2018

More-completely test <img srcset> selection on sizes attribute web-platform-tests/wpt#10392

Open

zcorpan mentioned this pull request Apr 26, 2019

Should support all CSS math functions in <img sizes> validator/validator#803

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3003] Allow mathematical expressions in image sizes #3084

[3003] Allow mathematical expressions in image sizes #3084

musgravejw commented Sep 30, 2017 •

edited by pr-preview bot

Loading

annevk commented Sep 30, 2017

musgravejw commented Oct 2, 2017

annevk commented Oct 2, 2017

zcorpan commented Oct 4, 2017

zcorpan Oct 4, 2017

zcorpan Oct 4, 2017

zcorpan commented Oct 4, 2017

annevk commented Oct 9, 2017

zcorpan commented Oct 9, 2017

annevk Oct 9, 2017

musgravejw commented Oct 11, 2017

musgravejw commented Oct 11, 2017

annevk commented Oct 11, 2017

musgravejw commented Oct 17, 2017

donovanglover commented Mar 2, 2018

musgravejw commented Mar 2, 2018

domfarolino commented Mar 23, 2018 •

edited

Loading

donovanglover commented Mar 24, 2018

domfarolino commented Mar 24, 2018

domfarolino commented Mar 25, 2018

annevk commented Mar 26, 2018

domfarolino commented Mar 26, 2018 •

edited

Loading

musgravejw commented Mar 27, 2018

annevk commented Apr 9, 2018

[3003] Allow mathematical expressions in image sizes #3084

[3003] Allow mathematical expressions in image sizes #3084

Conversation

musgravejw commented Sep 30, 2017 • edited by pr-preview bot Loading

annevk commented Sep 30, 2017

musgravejw commented Oct 2, 2017

annevk commented Oct 2, 2017

zcorpan commented Oct 4, 2017

zcorpan Oct 4, 2017

Choose a reason for hiding this comment

zcorpan Oct 4, 2017

Choose a reason for hiding this comment

zcorpan commented Oct 4, 2017

annevk commented Oct 9, 2017

zcorpan commented Oct 9, 2017

annevk Oct 9, 2017

Choose a reason for hiding this comment

musgravejw commented Oct 11, 2017

musgravejw commented Oct 11, 2017

annevk commented Oct 11, 2017

musgravejw commented Oct 17, 2017

donovanglover commented Mar 2, 2018

musgravejw commented Mar 2, 2018

domfarolino commented Mar 23, 2018 • edited Loading

donovanglover commented Mar 24, 2018

domfarolino commented Mar 24, 2018

domfarolino commented Mar 25, 2018

annevk commented Mar 26, 2018

domfarolino commented Mar 26, 2018 • edited Loading

musgravejw commented Mar 27, 2018

annevk commented Apr 9, 2018

musgravejw commented Sep 30, 2017 •

edited by pr-preview bot

Loading

domfarolino commented Mar 23, 2018 •

edited

Loading

domfarolino commented Mar 26, 2018 •

edited

Loading