[webkit-reviews] review granted: [Bug 226156] [LFC][TFC] Percent height resolving quirk should stop at the table formatting context root : [Attachment 429468] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun May 23 15:26:50 PDT 2021


Darin Adler <darin at apple.com> has granted zalan <zalan at apple.com>'s request for
review:
Bug 226156: [LFC][TFC] Percent height resolving quirk should stop at the table
formatting context root
https://bugs.webkit.org/show_bug.cgi?id=226156

Attachment 429468: Patch

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




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 429468
  --> https://bugs.webkit.org/attachment.cgi?id=429468
Patch

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

> Source/WebCore/layout/formattingContexts/table/TableFormattingQuirks.cpp:65
> +	   return height.isFixed() ? makeOptional(LayoutUnit { height.value()
}) : WTF::nullopt;

Instead of this:

    makeOptional(LayoutUnit { height.value() })

Does this compile?

    Optional<LayoutUnit> { height.value() }

I think I would prefer that.

> Source/WebCore/layout/formattingContexts/table/TableFormattingQuirks.cpp:70
> +    for (auto* ancestor = &layoutBox.containingBlock(); ancestor && ancestor
!= &tableBox; ancestor = &ancestor->containingBlock()) {

seems like we write this idiomatic loop so often; wish there was a cleaner
idiom for it, like we have for DOM tree ancestry:


More information about the webkit-reviews mailing list