[webkit-reviews] review granted: [Bug 195417] [ContentChangeObserver] Check if max-height change triggers visible content change. : [Attachment 363897] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted  review:
Bug 195417: [ContentChangeObserver] Check if max-height change triggers visible
content change.
https://bugs.webkit.org/show_bug.cgi?id=195417

Attachment 363897: Patch

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




--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 363897
  --> https://bugs.webkit.org/attachment.cgi?id=363897
Patch

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

> Source/WebCore/page/ios/ContentChangeObserver.cpp:299
>      if (left.isFixed() && width.isFixed() && -left.value() >= width.value())

This is stupid and you should add a comment to explain how stupid it is.

> Source/WebCore/page/ios/ContentChangeObserver.cpp:305
> +    auto maxHeight = style.maxHeight();

Maybe comment to say that height + overflow:hidden should also be checked at
some point.