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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=195180

Antti Koivisto <koivisto at iki.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |koivisto at iki.fi
 Attachment #395487|review?                     |review+
              Flags|                            |

--- Comment #11 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 395487
  --> https://bugs.webkit.org/attachment.cgi?id=395487
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

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200406/4f4d5605/attachment.htm>