[Webkit-unassigned] [Bug 224185] Interoperability issue in margin collapsing with overflow:hidden elements

Tue Apr 6 09:03:13 PDT 2021

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

--- Comment #12 from zalan <zalan at apple.com> ---
(In reply to Sergio Villar Senin from comment #11)
> (In reply to zalan from comment #10)
> > (In reply to zalan from comment #9)
> > > Created attachment 425229 [details]
> > > Safari-Chrome-LFC
> > > 
> > > So it looks LFC collapses these margins properly.
> > and all the other cases are working as well.
> 
> Nice!
> 
> BTW this is the patch I was working on:
> 
> @@ -497,7 +497,8 @@ bool RenderBlock::isSelfCollapsingBlock() const
>      if (logicalHeight() > 0
>          || isTable() || borderAndPaddingLogicalHeight()
>          || style().logicalMinHeight().isPositive()
> -        || style().marginBeforeCollapse() == MarginCollapse::Separate ||
> style().marginAfterCollapse() == MarginCollapse::Separate)
> +        || style().marginBeforeCollapse() == MarginCollapse::Separate ||
> style().marginAfterCollapse() == MarginCollapse::Separate
> +        || createsNewFormattingContext())
>          return false;
>  
> @@ -4675,7 +4675,7 @@ bool RenderBox::shrinkToAvoidFloats() const
>  
>  bool RenderBox::createsNewFormattingContext() const
>  {
> -    return isInlineBlockOrInlineTable() ||
> isFloatingOrOutOfFlowPositioned() || hasOverflowClip() ||
> isFlexItemIncludingDeprecated()
> +    return isInlineBlockOrInlineTable() ||
> isFloatingOrOutOfFlowPositioned() || hasOverflowClip() ||
> isFlexItemIncludingDeprecated() || !style().isOverflowVisible()
>          || isTableCell() || isTableCaption() || isFieldset() ||
> isWritingModeRoot() || isDocumentElementRenderer() ||
> isRenderFragmentedFlow() || isRenderFragmentContainer()
>          || isGridItem() || style().specifiesColumns() ||
> style().columnSpan() == ColumnSpan::All || style().display() ==
> DisplayType::FlowRoot;
> 
> This fixes all the cases except the ones involving <body>. As I mentioned
> the problem is that style().isOverflowVisible() is not exactly what we want.
> We need something like
> https://webkit-search.igalia.com/webkit/source/Source/WebCore/layout/
> layouttree/LayoutBox.cpp#391 which you added in r240213.
> 
> I wonder what's the plan from the LFC POV. Is it OK to share code with the
> good-old-layout system and LFC? Would it be better to reimplement it so it
> does not taint LFC?
I don't think it's worth templatizing it (or share this logic through some other means) I'd just reimplement it in RenderElement (or wherever it is applicable).

-- 
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/20210406/ea3f60cf/attachment-0001.htm>