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

Tue Apr 6 01:02:51 PDT 2021

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

--- Comment #11 from Sergio Villar Senin <svillar at igalia.com> ---
(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?

-- 
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/9028e3f6/attachment.htm>
