Summary: | font-size with viewport units in calc() doesn't change when viewport resizes | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sara Soueidan <sara.soueidan> | ||||
Component: | Layout and Rendering | Assignee: | Darin Adler <darin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bfulgham, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, heycam, macpherson, menard, mmaxfield, rik, simon.fraser, twilco.o, webkit-bug-importer, zalan | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | Safari 14 | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Sara Soueidan
2021-04-15 10:59:05 PDT
Thanks for the report, Sara! sure thing! I hope this gets some attention and a fix soon <3 (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. 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(). Title should maybe say "viewport units in calc expressions" Great to have that super-reduced test case 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. 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. (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. Found out CSSToLengthConversionData already does this "watch for viewport-dependent values" thing, just turns it off for font-size! I’ve got a working fix for this, but need to construct some regression tests. Created attachment 426241 [details]
Patch
Patch above includes a regression test so this should be good to go Alan, thanks so much for reviewing! Committed r276187 (236669@main): <https://commits.webkit.org/236669@main> |