WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
224614
font-size with viewport units in calc() doesn't change when viewport resizes
https://bugs.webkit.org/show_bug.cgi?id=224614
Summary
font-size with viewport units in calc() doesn't change when viewport resizes
Sara Soueidan
Reported
2021-04-15 10:59:05 PDT
Note: this bug is identical to the Issue #124331 which is currently closed and marked as fixed. So I am borrowing parts of the description from that issue. What steps will reproduce the problem? 1. Set font-size on h1 to any value using a viewport unit inside a custom property. 2. resize browser window 3. font size doesn't change along with new viewport width, you have to refresh page. What is the expected result? The font should resize in response to the viewport width change What happens instead? The font doesn't resize Live reduced test case:
https://codepen.io/ausi/pen/WNRyOrY
The reduced test case includes a fix using -webkit-marquee-increment: 0vw; The issue can also be worked around using `min-height: 1vw` on the element itself or a container of that element (such as the body element).
Attachments
Patch
(16.10 KB, patch)
2021-04-16 09:46 PDT
,
Darin Adler
zalan
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2021-04-15 11:16:11 PDT
Thanks for the report, Sara!
Radar WebKit Bug Importer
Comment 2
2021-04-15 11:16:21 PDT
<
rdar://problem/76713459
>
Sara Soueidan
Comment 3
2021-04-15 11:17:55 PDT
sure thing! I hope this gets some attention and a fix soon <3
Darin Adler
Comment 4
2021-04-15 12:50:36 PDT
(In reply to Sara Soueidan from
comment #0
)
> Note: this bug is identical to the Issue #124331 which is currently closed > and marked as fixed.
Was looking for this and couldn’t find it.
Darin Adler
Comment 5
2021-04-15 12:53:45 PDT
Looking at the test case: While not mentioned above in the summary, when calc() is not involved, this works as expected. So it’s somehow specific to calc().
Darin Adler
Comment 6
2021-04-15 12:54:09 PDT
Title should maybe say "viewport units in calc expressions"
Darin Adler
Comment 7
2021-04-15 12:55:52 PDT
Great to have that super-reduced test case
Darin Adler
Comment 8
2021-04-15 13:00:54 PDT
Looks like the bug may be in BuilderCustom::applyValueFontSize: if (primitiveValue.isViewportPercentageLength()) builderState.style().setHasViewportUnits(); There’s no similar call to setHasViewportUnits in the isCalculatedPercentageWithLength case just below.
Darin Adler
Comment 9
2021-04-15 14:10:08 PDT
It’s not about isCalculatedPercentageWithLength, I don’t think, although that may be a more obscure case. It’s simply that a calculated length currently can’t answer the "has viewport units" question. So I see 3 options: 1) Call setHasViewportUnits for all of them, potentially making resizing unnecessarily slow on all websites that use calc() but not things that are viewport-dependent. 2) Add code in calculated value parsing to compute "is viewport dependent" as we go, and store that information in CalculationValue and CSSCalcValue objects. 3) Record whether a calculated value turned out to be viewport dependent as a side effect of performing a calculation and read that boolean out after calling computeLength. Could possibly be stored in CSSToLengthConversionData.
Darin Adler
Comment 10
2021-04-15 14:11:09 PDT
(In reply to Darin Adler from
comment #9
)
> 3) Record whether a calculated value turned out to be viewport dependent as > a side effect of performing a calculation and read that boolean out after > calling computeLength. Could possibly be stored in CSSToLengthConversionData.
Would have to make CSSToLengthConversionData non-const, though.
Darin Adler
Comment 11
2021-04-15 15:08:33 PDT
Found out CSSToLengthConversionData already does this "watch for viewport-dependent values" thing, just turns it off for font-size!
Darin Adler
Comment 12
2021-04-16 08:30:17 PDT
I’ve got a working fix for this, but need to construct some regression tests.
Darin Adler
Comment 13
2021-04-16 09:46:18 PDT
Created
attachment 426241
[details]
Patch
Darin Adler
Comment 14
2021-04-16 15:35:27 PDT
Patch above includes a regression test so this should be good to go
Darin Adler
Comment 15
2021-04-16 18:06:28 PDT
Alan, thanks so much for reviewing!
Darin Adler
Comment 16
2021-04-16 18:08:52 PDT
Committed
r276187
(
236669@main
): <
https://commits.webkit.org/236669@main
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug