Bug 195180 – [css-values-4] Support font-relative lh and rlh unit

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Gérard Talbot (no longer involved)]

CSS4 Values and Units, section 6.1.1 Font-relative lengths: the lh, rlh units
https://www.w3.org/TR/css-values-4/#font-relative-lengths
states

"
lh unit
    Equal to the computed value of the line-height property of the element on which it is used, converting normal to an absolute length by using only the metrics of the first available font. 

rlh unit
    Equal to the computed value of line-height property on the root element, converting normal to an absolute length as above.
"

Tests
-----

http://w3c-test.org/css/css-values/lh-rlh-on-root-001.html

http://w3c-test.org/css/css-values/lh-unit-001.html

http://w3c-test.org/css/css-values/lh-unit-002.html


Test results
- - - - - -

https://wpt.fyi/results/css/css-values?label=master

So far, no browser pass these 3 tests.


Correspondent Bugzilla Mozilla bug report:
https://bugzilla.mozilla.org/show_bug.cgi?id=1310170

[Radar WebKit Bug Importer]

<rdar://problem/48532318>

[Tyler Wilcock]

Hi!  Looks like there is a duplicate of this -- https://bugs.webkit.org/show_bug.cgi?id=204277.

[Simon Fraser (smfr)]

*** Bug 204277 has been marked as a duplicate of this bug. ***

[Tyler Wilcock]

Created attachment 395461 [details]
Patch

[Tyler Wilcock]

Hello!  This is my first patch -- hope I've done everything correct.  I look forward to your review.

[Tyler Wilcock]

Created attachment 395462 [details]
Patch

[Tyler Wilcock]

Created attachment 395473 [details]
Patch

[Tyler Wilcock]

Created attachment 395479 [details]
Patch

[Tyler Wilcock]

Created attachment 395487 [details]
Patch

[Tyler Wilcock]

Sorry for the churn — should be all set for review now.

[Antti Koivisto]

Comment on attachment 395487 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395487&action=review

Looks good!

> Source/WebCore/css/CSSToLengthConversionData.h:44
> +enum class ComputingProperty {
> +    FontSize,
> +    LineHeight,
> +    OtherOrUnknown
> +};

An alternative approach would be to use Optional<CSSPropertyID> instead of this (passing CSSPropertyFontSize/CSSPropertyLineHeight where appropriate).

If you keep the enum it should move to CSSToLengthConversionData namespace. PropertyToCompute or just PropertyType might read better.

> Source/WebCore/css/CSSToLengthConversionData.h:48
> +    CSSToLengthConversionData(const RenderStyle* style, const RenderStyle* rootStyle, const RenderStyle* parentStyle, const RenderView* renderView, float zoom, ComputingProperty computingProperty = ComputingProperty::OtherOrUnknown)

'computingProperty' is an awkward name for variables and fields too

[Tyler Wilcock]

Created attachment 395653 [details]
Patch

[Tyler Wilcock]

Thanks for the suggestions, I've applied them in this new patch.

[Antti Koivisto]

Thanks for the patch!

[EWS]

Committed r259703: <https://trac.webkit.org/changeset/259703>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395653 [details].

[Simon Fraser (smfr)]

This was moved behind an off-by-default flag:
https://bugs.webkit.org/show_bug.cgi?id=211356