Bug 87846 - vw/vh units used as font/line-height values don't scale with the viewport
: vw/vh units used as font/line-height values don't scale with the viewport
Status: NEW
: WebKit
Layout and Rendering
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
: 131552
: 124052
  Show dependency treegraph
 
Reported: 2012-05-30 05:32 PST by
Modified: 2014-04-16 14:20 PST (History)


Attachments
testcase (541 bytes, text/html)
2012-08-20 11:41 PST, Ojan Vafai
no flags Details
Patch (14.09 KB, patch)
2013-03-19 00:06 PST, Timothy Loh
no flags Review Patch | Details | Formatted Diff | Diff
Patch (14.80 KB, patch)
2013-03-21 16:35 PST, Timothy Loh
no flags Review Patch | Details | Formatted Diff | Diff
Uber patch (282.80 KB, patch)
2014-04-11 12:12 PST, Bem Jones-Bey
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-05-30 05:32:36 PST
While experimenting with vh/vw units in Chrome/Canary, I tried assigning line-height a vw unit side, which worked successfully, but the size didn't increase/decrease as the viewport did, unlike padding which increased/decreased as expected. Font size doesn't scale either. Padding and widths do. I believe the expected behavior is for anything sized using vh/vw units to scale with viewport, not just width/margin/padding.
------- Comment #1 From 2012-05-30 06:28:50 PST -------
(In reply to comment #0)
> While experimenting with vh/vw units in Chrome/Canary, I tried assigning line-height a vw unit side, which worked successfully, but the size didn't increase/decrease as the viewport did, unlike padding which increased/decreased as expected. Font size doesn't scale either. Padding and widths do. I believe the expected behavior is for anything sized using vh/vw units to scale with viewport, not just width/margin/padding.

Upon more experimenting, it seems as if only padding/margin scale as viewport does; height and width do not.
------- Comment #2 From 2012-08-20 09:26:50 PST -------
I can confirm this bug. Reported at www-style by Giuseppe Bilotta<giuseppe.bilotta@gmail.com>;
------- Comment #3 From 2012-08-20 10:07:36 PST -------
Link to the discussion on www-style (with test cases): http://lists.w3.org/Archives/Public/www-style/2012Aug/0567.html
------- Comment #4 From 2012-08-20 11:41:19 PST -------
Created an attachment (id=159480) [details]
testcase
------- Comment #5 From 2013-03-19 00:06:27 PST -------
Created an attachment (id=193747) [details]
Patch
------- Comment #6 From 2013-03-19 00:19:42 PST -------
The issue the above patch should fix is that we resolve the font-size and line-height values when we calculate styles, and we don't currently recalculate styles upon resize. The test case Ojan has attached is a slightly different issue arising from the code being designed with some no longer valid assumptions about font sizes and zoom. For fonts, we store a specified size (i.e. the intended pixel size), and a computed size (which takes zoom into account, as well as minimum and maximum font sizes). The calculation for the computed size simply takes the specified size so font sizes defined in viewport units still gets scaled.
------- Comment #7 From 2013-03-21 16:35:10 PST -------
Created an attachment (id=194379) [details]
Patch
------- Comment #8 From 2013-03-27 17:51:13 PST -------
(From update of attachment 194379 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=194379&action=review

> Source/WebCore/css/CSSGrammar.y.in:1761
> +      if (parser->m_styleSheet)
> +          parser->m_styleSheet->parserSetUsesViewportUnits(true);

It's not clear to me that this should be a side-effect of parsing.  This is a side-effect of parsing that we only sometimes want, right?
------- Comment #9 From 2013-03-27 17:52:47 PST -------
(From update of attachment 194379 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=194379&action=review

>> Source/WebCore/css/CSSGrammar.y.in:1761
>> +          parser->m_styleSheet->parserSetUsesViewportUnits(true);
> 
> It's not clear to me that this should be a side-effect of parsing.  This is a side-effect of parsing that we only sometimes want, right?

For example, getComputedStyle presumably calls into the CSS parser but wouldn't want side-effects like this to occur.  I guess it wouldn't have an m_styleSheet set at the time?
------- Comment #10 From 2013-03-27 18:05:47 PST -------
(From update of attachment 194379 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=194379&action=review

>>> Source/WebCore/css/CSSGrammar.y.in:1761
>>> +          parser->m_styleSheet->parserSetUsesViewportUnits(true);
>> 
>> It's not clear to me that this should be a side-effect of parsing.  This is a side-effect of parsing that we only sometimes want, right?
> 
> For example, getComputedStyle presumably calls into the CSS parser but wouldn't want side-effects like this to occur.  I guess it wouldn't have an m_styleSheet set at the time?

I think having this for any viewport units in a document means we'll be guaranteed to have viewport size information when we resolve CSSPrimitiveValues into  Length objects, which will mean we won't need viewport unit types for Length objects (sorry this isn't explicitly mentioned in the ChangeLog, I hadn't thought of it earlier). So while for this particular issue we'll only want it for font sizes and line heights, it seems like it'll make getting viewport units working with everything much simpler. Having viewport units go through the parser without actually using them doesn't seem like a case worth optimising right now.
------- Comment #11 From 2014-04-11 12:12:26 PST -------
Created an attachment (id=229147) [details]
Uber patch

Full change, to help explain the patch in Bug 131552