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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 7 11:40:43 PST 2019

Simon Fraser (smfr) <simon.fraser at apple.com> has granted  review:
Bug 195417: [ContentChangeObserver] Check if max-height change triggers visible
content change.

Attachment 363897: Patch


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

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.

More information about the webkit-reviews mailing list