Calc function in sizes attribute doesn't work correctly (probably operator precendence problem)
Reported by
radek.s...@gmail.com,
Jan 19 2018
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36 Steps to reproduce the problem: 1. Navigate to https://codepen.io/anon/pen/GyPymY?editors=1100#0 2. Change window size to something between 1000px and 1400px 3. Reload the page. What is the expected behavior? All three images should use the same image size (430x330) as mathematically, all expressions in calc functions are equivalent. What went wrong? The first image is loaded in bigger resolution than necessary. It should have loaded 430x330 as well, but loaded 645x495. Only difference are omitted parentheses in calc expression around multiplication. Did this work before? N/A Does this work in other browsers? Yes Chrome version: 63.0.3239.132 Channel: stable OS Version: 10.0 Flash Version:
,
Jan 22 2018
Able to reproduce the issue on reported chrome version 63.0.3239.132 and on the latest canary 66.0.3327.0 using Mac 10.13.1 , Windows 10 and Ubuntu 14.04. As per comment#1 suspecting the same i.e., Change log: https://chromium.googlesource.com/chromium/src/+log/70737eee..9a186f77?pretty=fuller and Suspecting https://chromium.googlesource.com/chromium/blink/+/390858b22f2f453a68a2e56150a0f255e61a1c96 Review URL: https://codereview.chromium.org/1045353002 @yoav@yoav: Please feel free to re-assign if it is not related to your change. Adding RB-Stable, Please feel free to remove if it is not applicable. Thanks!
,
Jan 22 2018
,
Jan 22 2018
I don't know if it's related to that change (I'll have to take a deeper look), but according to your bisect, this issue has been around for 23 (!!!) Chrome releases and almost 3 years. Any particular reason you think this should be is a P1 ReleaseBlocker-Stable?
,
Jan 23 2018
,
Jan 24 2018
That blame is almost certainly due to something changing the image behavior that revealed the calc problem. Isn't calc a CSS thing?
,
Jan 24 2018
But in any event, yoav@, could you check the chain of values going into the image selection to see where it goes wrong (i.e the CSS or the length resolution or what)?
,
Jan 24 2018
,
Jan 26 2018
There does indeed appear to be an operator precedence issue in the 'sizes' calc() parser. Should have a fix shortly.
,
Jan 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e0bcd910832c55445bc6bb4156acd716e0667248 commit e0bcd910832c55445bc6bb4156acd716e0667248 Author: Fredrik Söderquist <fs@opera.com> Date: Sat Jan 27 00:31:36 2018 Fix operator precedence in SizesCalcParser Pop all operators with greater/equal precedence rather than just the top-most. Bug: 803824 Change-Id: I5f106a0e82ed98ba2510c86ed157bf4b133cb5b8 Reviewed-on: https://chromium-review.googlesource.com/888919 Commit-Queue: Fredrik Söderquist <fs@opera.com> Reviewed-by: Yoav Weiss <yoav@yoav.ws> Cr-Commit-Position: refs/heads/master@{#532100} [modify] https://crrev.com/e0bcd910832c55445bc6bb4156acd716e0667248/third_party/WebKit/Source/core/css/parser/SizesCalcParser.cpp [modify] https://crrev.com/e0bcd910832c55445bc6bb4156acd716e0667248/third_party/WebKit/Source/core/css/parser/SizesCalcParserTest.cpp
,
Jan 27 2018
Operator precedence issue fixed - I'm not ruling out that there might be a image selection issue, but it seems better to handle that in a different bug if so. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by woxxom@gmail.com
, Jan 19 2018