[webkit-reviews] review granted: [Bug 227505] Move BottomControlsBarHeight and InsideMargin to be computed at runtime : [Attachment 432650] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Dean Jackson <dino at apple.com>'s
request for review:
Bug 227505: Move BottomControlsBarHeight and InsideMargin to be computed at
runtime
https://bugs.webkit.org/show_bug.cgi?id=227505

Attachment 432650: Patch

https://bugs.webkit.org/attachment.cgi?id=432650&action=review




--- Comment #6 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 432650
  --> https://bugs.webkit.org/attachment.cgi?id=432650
Patch

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

r=mews

>
Source/WebCore/Modules/modern-media-controls/controls/inline-media-controls.js:
133
> +	   const inlineInsideMargin =
this.computedValueForStylePropertyInPx("--inline-controls-inside-margin");

Can we maybe cache this value somewhere so that we don't have to get the
computed style more than once?	We don't expect this value to change so there's
not much benefit to requesting it more than once.

>
Source/WebCore/Modules/modern-media-controls/controls/inline-media-controls.js:
219
> +	   const inlineBottomControlsBarHeight =
this.computedValueForStylePropertyInPx("--inline-controls-bar-height");

ditto (:133)

> Source/WebCore/Modules/modern-media-controls/controls/layout-node.js:241
> +	       return null;

NIT: maybe the fallback value should be `0` so that it's still valid px value
(and therefore able to be used in math operations)?

> Source/WebCore/Modules/modern-media-controls/controls/layout-node.js:244
> +	   return parseFloat(value);

Not sure which is better, but you also could do this with
```
   
window.getComputedStyle(this.element).getPropertyCSSValue(propertyName).getFloa
tValue(CSSPrimitiveValue.CSS_PX)
```

>
Source/WebCore/Modules/modern-media-controls/controls/macos-inline-media-contro
ls.js:63
> +	   const inlineInsideMargin =
this.computedValueForStylePropertyInPx("--inline-controls-inside-margin");
> +	   const inlineBottomControlsBarHeight =
this.computedValueForStylePropertyInPx("--inline-controls-bar-height");

ditto (inline-media-controls.js:133)