Bug 244978 – [css-contain] Fix contain-size-monolithic-002.html

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Rob Buis]

Fix contain-size-monolithic-002.html.

[Rob Buis]

Created attachment 462230 [details]
Patch

[Tim Nguyen (:ntim)]

Is this is the same bug as bug 238854?

[Tim Nguyen (:ntim)]

Bug 238854 has a testcase of the bug without CSS containment, so ideally the fix would be fixing that testcase as well.

[Tim Nguyen (:ntim)]

Bug 41796 is related as well.

[Rob Buis]

Created attachment 462295 [details]
Patch

[Rob Buis]

Created attachment 462319 [details]
Patch

[Rob Buis]

(In reply to Tim Nguyen (:ntim) from comment #4)
> Bug 41796 is related as well.

Thanks Tim, I knew about this one but not bug 238854.

[Rob Buis]

@Alan I noticed fragmentedFlowPortionOverflowRect does not include column gap. My patch adds the column gap later on to paginationClip so we include the column gap when clipping for painting columns in RenderLayer and thus fixes contain-size-monolithic-002.html and bug 41796 test case. However I do not know this code very well. In general, do you know how clipping should work for multicol? For example I notice for some reason the bug 238854 test case is not clipped to the total column bounds in chromium, but is in Firefox. I also wonder if out of flow content is clipped at all in multcol.

[alan baradlay]

(In reply to Rob Buis from comment #8)
> @Alan I noticed fragmentedFlowPortionOverflowRect does not include column
> gap. My patch adds the column gap later on to paginationClip so we include
> the column gap when clipping for painting columns in RenderLayer and thus
> fixes contain-size-monolithic-002.html and bug 41796 test case. However I do
> not know this code very well. In general, do you know how clipping should
> work for multicol? 
No, I don't know too much about clipping in general. Maybe Simon?

For example I notice for some reason the bug 238854 test
> case is not clipped to the total column bounds in chromium, but is in
> Firefox. I also wonder if out of flow content is clipped at all in multcol.
Not sure but I'd think it's relatively easy to create a test case to confirm it.

[Rob Buis]

Layer tree without the position: relative:
layer 0x143003400 scrollableArea 0x10504d6c0 at (0,0) size 938x753 (composited [root], bounds=at (0,0) size 938x753, drawsContent=1, paints into ancestor=0)
  RenderView 0x143003120 at (0,0) size 938x753
 positive z-order list (1)
  layer 0x143000b90 scrollableArea 0x10504d940 at (0,0) size 938x58
    RenderBlock 0x143001860 {HTML} at (0,0) size 938x58
      RenderBody 0x143001ad0 {BODY} at (8,16) size 922x34
        RenderBlock 0x143005060 {P} at (0,0) size 922x18
          RenderText 0x1430051a0 {#text} at (0,0) size 274x18
            text run at (0,0) width 274: "Test passes if there is a filled green square."
   normal flow list (1)
    layer 0x1430019a0 at (8,50) size 300x0
      RenderBlock 0x143005230 {DIV} at (0,34) size 300x0 id="multicol"
        RenderMultiColumnSet 0x143005a60 at (0,0) size 300x0
     normal flow list (1)
      layer 0x143005930 at (8,50) size 89x0
        RenderMultiColumnFlowThread 0x143005720 at (0,0) size 90x0
          RenderBlock 0x143005370 {DIV} at (0,0) size 90x0 id="container"
   positive z-order list (1)
    layer 0x1430055f0 at (8,50) size 100x100
      RenderBlock (positioned) 0x1430054b0 {DIV} at (8,50) size 100x100 [bgcolor=#008000] id="abs-size-contain"

with position: relative:
layer 0x135003400 scrollableArea 0x10504f380 at (0,0) size 938x753 (composited [root], bounds=at (0,0) size 938x753, drawsContent=1, paints into ancestor=0)
  RenderView 0x135003120 at (0,0) size 938x753
 positive z-order list (1)
  layer 0x135000b90 scrollableArea 0x10504f600 at (0,0) size 938x158
    RenderBlock 0x135001860 {HTML} at (0,0) size 938x158
      RenderBody 0x135001ad0 {BODY} at (8,16) size 922x134
        RenderBlock 0x135005060 {P} at (0,0) size 922x18
          RenderText 0x1350051a0 {#text} at (0,0) size 274x18
            text run at (0,0) width 274: "Test passes if there is a filled green square."
   normal flow list (1)
    layer 0x1350019a0 at (8,50) size 300x100
      RenderBlock 0x135005230 {DIV} at (0,34) size 300x100 id="multicol"
        RenderMultiColumnSet 0x135005b90 at (0,0) size 300x100
     normal flow list (1)
      layer 0x135005a60 at (8,50) size 89x0
        RenderMultiColumnFlowThread 0x135005850 at (0,0) size 90x0 (layout overflow 0,0 100x100)
   positive z-order list (2)
    layer 0x1350054b0 at (8,50) size 89x0
      RenderBlock (relative positioned) 0x135005370 {DIV} at (0,0) size 90x0 id="container" (layout overflow 0,0 100x100)
    layer 0x135005720 at (8,50) size 100x100
      RenderBlock (positioned) 0x1350055e0 {DIV} at (0,0) size 100x100 [bgcolor=#008000] id="abs-size-contain"

[Radar WebKit Bug Importer]

<rdar://problem/100026789>

[Rob Buis]

Created attachment 462399 [details]
Patch

[Rob Buis]

Created attachment 462407 [details]
Patch

[Tim Nguyen (:ntim)]

The testcase in bug 238854 still doesn't look quite right with the patch applied. The green area looks shorter than other browsers.

[Tim Nguyen (:ntim)]

Created attachment 462412 [details]
Screenshot of testcase in bug 238854

I think we might need to expand beyond the colGap, but to the width of the largest item in the column.

[alan baradlay]

Comment on attachment 462407 [details]
Patch

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

> COMMIT_MESSAGE:6
> +Include the column gap when calculating the pagination clip.

This comment is rather redundant.

[Rob Buis]

Comment on attachment 462407 [details]
Patch

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

>> COMMIT_MESSAGE:6
>> +Include the column gap when calculating the pagination clip.
> 
> This comment is rather redundant.

Thanks for the review! Unfortunately, now that I look at it again, it is probably not the right fix, as the test case will fail if the div is made wider (it would still be clipped at 100px). I think the real fix is https://bugs.webkit.org/show_bug.cgi?id=238854.

[Rob Buis]

Ths is fixed by the bug 238854 fix.