Bug 246060 – threejs.org renders mostly blank (lazy image loading inside 0-height inline is broken?)

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Karl Dubost]

Steps to reproduce:
1. With Safari 16.1 (18614.2.6.1.1) on macOS
2. Go to https://threejs.org

Expected:
The right side of the screen (`#projects`) is full of square images

Actual:
The area is empty. 
`#projects` has a 0px height. Images have not been loaded. 


Each images are 

```
<a href="https://eyes.nasa.gov/apps/mars2020/" target="_blank" rel="noopener"><img src="files/projects/mars2020.png" loading="lazy"></a>
```

if I toggle off and on:

```
#projects a {
overflow: hidden;
}
```

or 

```
#projects {
line-height: 0;
}
```

The images are being downloaded and the grid appears.

This is working as expected in Firefox Nightly 107 and Chrome Canary 108.

also on rdar://99905638

[Rob Buis]

I am not sure if this is a regression, I see the same behaviour on 15.4.
(I am back next week so can take a look then).

[Alexey Proskuryakov]

I can also reproduce with older Safari, 15.6.1 on macOS Monterey.

[Rob Buis]

Created attachment 462924 [details]
Patch

[Rob Buis]

The problem seems to be that intersection observer is not triggering the images as intersecting the root, so no lazy loads are triggered. This seems to depend on the line-height in combination with overflow: hidden; on my test line-height <= 0.5 causes images to register as non intersecting, bigger values do register as intersecting and thus trigger the lazy loads.

[Rob Buis]

One possible cause, take this snippet:

<div style="display:grid; grid-template-columns: repeat(7, 1fr);line-height:0">
  <div style="overflow:hidden"><img/></div>
</div>

In WebKit the inner div as well as the grid div get assigned height of 0, whereas in Firefox/Chrome they get 4/5 pixels instead. The larger area obviously has more of a chance of causing an intersection.

[Simon Fraser (smfr)]

Ignoring grid layout behavior, how does lazy-load of an image inside a height:0 container ever work?

[Rob Buis]

(In reply to Simon Fraser (smfr) from comment #6)
> Ignoring grid layout behavior, how does lazy-load of an image inside a
> height:0 container ever work?

For example:
  <div style="width:0px;height:0px">
    <img loading="lazy" src="https://threejs.org/files/projects/junni.png"/>
  </div>

Here the 0x0 image inside the 0x0 container still intersects.

[Simon Fraser (smfr)]

Does this work because of edge-inclusive intersection?

[Rob Buis]

(In reply to Simon Fraser (smfr) from comment #8)
> Does this work because of edge-inclusive intersection?

Yes, VisibleRectContextOption::UseEdgeInclusiveIntersection is used in Document.cpp:8169 as well as in computeClippedRectInRootContentsSpace and Document.cpp:8177 uses edgeInclusiveIntersect.

[Rob Buis]

The lazy loading starts to work when IFC internal feature is disabled.

[Rob Buis]

Created attachment 463466 [details]
Reduced test case

Reduced test case.

[Rob Buis]

(In reply to Rob Buis from comment #10)
> The lazy loading starts to work when IFC internal feature is disabled.

The first grid layout for the reduced test case with IFC enabled is:

(B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y, (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer, hasLayer(S)crollableArea, (C)omposited, (+)Dirty style, (+)Dirty layout
B---YGLSC -+  RenderView at (0,0) size 1728x0 renderer (0x1440008d0) layout box (0x0) layout->[normal child]
B-----LS- -+    HTML RenderBlock at (0,0) size 1728x0 renderer (0x144001920) layout box (0x0) node (0x144001240) layout->[self][normal child]
B-------- -+      BODY RenderBody at (0,8) size 1712x0 renderer (0x144001ba0) layout box (0x0) node (0x144001480) layout->[self][normal child]
B-------- --*       DIV RenderGrid at (0,0) size 1712x0 renderer (0x144004760) layout box (0x0) node (0x144004200)
B--O--LS- --          A RenderBlock at (0,0) size 1712x0 renderer (0x144004ab0) layout box (0x144004e00) node (0x144004400)
-------- --            line at (0.00,0.00) size (1712.00x0.00) baseline (5.00) enclosing top (5.00) bottom (5.00)
-------- --              Root inline box at (0.00,-9.00) size (0.00x18.00)
-------- --              Run(s):
-------- --                Atomic box at (0.00,5.00) size 0.00x0.00 renderer->(0x144004c00)
I-------- --            IMG RenderImage at (0,5) size 0x0 renderer (0x144004c00) layout box (0x144004ed0) node (0x144004540)

With IFC disabled:
(B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y, (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer, hasLayer(S)crollableArea, (C)omposited, (+)Dirty style, (+)Dirty layout
B---YGLSC -+  RenderView at (0,0) size 1728x0 renderer (0x1440008d0) layout box (0x0) layout->[normal child]
B-----LS- -+    HTML RenderBlock at (0,0) size 1728x0 renderer (0x144001920) layout box (0x0) node (0x144001240) layout->[self][normal child]
B-------- -+      BODY RenderBody at (0,8) size 1712x0 renderer (0x144001ba0) layout box (0x0) node (0x144001480) layout->[self][normal child]
B-------- --*       DIV RenderGrid at (0,0) size 1712x5 renderer (0x144004760) layout box (0x0) node (0x144004200)
B--O--LS- --          A RenderBlock at (0,0) size 1712x5 renderer (0x144004ab0) layout box (0x0) node (0x144004400)
-------- --            Line: (top: -9 bottom: 9) with leading (top: 0 bottom: 5)
-------- --            RootInlineBox at (0,-9) size 0x18 (0x144007f20) renderer->(0x144004ab0)
-------- --              InlineBox at (0,5) size 0x0 (0x144007e80) renderer->(0x144004c00)
I-------- --            IMG RenderImage at (0,5) size 0x0 renderer (0x144004c00) layout box (0x0) node (0x144004540)

Only in the second situation is the lazy load processed. I think we have to find out first if the changed render tree with IFC is correct.

Note that FF and Chrome show the image when loading the reduced test case.

[alan]

(In reply to Rob Buis from comment #12)
> (In reply to Rob Buis from comment #10)
> > The lazy loading starts to work when IFC internal feature is disabled.
> 
> The first grid layout for the reduced test case with IFC enabled is:
> 
> (B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y,
> (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer,
> hasLayer(S)crollableArea, (C)omposited, (+)Dirty style, (+)Dirty layout
> B---YGLSC -+  RenderView at (0,0) size 1728x0 renderer (0x1440008d0) layout
> box (0x0) layout->[normal child]
> B-----LS- -+    HTML RenderBlock at (0,0) size 1728x0 renderer (0x144001920)
> layout box (0x0) node (0x144001240) layout->[self][normal child]
> B-------- -+      BODY RenderBody at (0,8) size 1712x0 renderer
> (0x144001ba0) layout box (0x0) node (0x144001480) layout->[self][normal
> child]
> B-------- --*       DIV RenderGrid at (0,0) size 1712x0 renderer
> (0x144004760) layout box (0x0) node (0x144004200)
> B--O--LS- --          A RenderBlock at (0,0) size 1712x0 renderer
> (0x144004ab0) layout box (0x144004e00) node (0x144004400)
> -------- --            line at (0.00,0.00) size (1712.00x0.00) baseline
> (5.00) enclosing top (5.00) bottom (5.00)
> -------- --              Root inline box at (0.00,-9.00) size (0.00x18.00)
> -------- --              Run(s):
> -------- --                Atomic box at (0.00,5.00) size 0.00x0.00
> renderer->(0x144004c00)
> I-------- --            IMG RenderImage at (0,5) size 0x0 renderer
> (0x144004c00) layout box (0x144004ed0) node (0x144004540)
> 
> With IFC disabled:
> (B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y,
> (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer,
> hasLayer(S)crollableArea, (C)omposited, (+)Dirty style, (+)Dirty layout
> B---YGLSC -+  RenderView at (0,0) size 1728x0 renderer (0x1440008d0) layout
> box (0x0) layout->[normal child]
> B-----LS- -+    HTML RenderBlock at (0,0) size 1728x0 renderer (0x144001920)
> layout box (0x0) node (0x144001240) layout->[self][normal child]
> B-------- -+      BODY RenderBody at (0,8) size 1712x0 renderer
> (0x144001ba0) layout box (0x0) node (0x144001480) layout->[self][normal
> child]
> B-------- --*       DIV RenderGrid at (0,0) size 1712x5 renderer
> (0x144004760) layout box (0x0) node (0x144004200)
> B--O--LS- --          A RenderBlock at (0,0) size 1712x5 renderer
> (0x144004ab0) layout box (0x0) node (0x144004400)
> -------- --            Line: (top: -9 bottom: 9) with leading (top: 0
> bottom: 5)
> -------- --            RootInlineBox at (0,-9) size 0x18 (0x144007f20)
> renderer->(0x144004ab0)
> -------- --              InlineBox at (0,5) size 0x0 (0x144007e80)
> renderer->(0x144004c00)
> I-------- --            IMG RenderImage at (0,5) size 0x0 renderer
> (0x144004c00) layout box (0x0) node (0x144004540)
> 
> Only in the second situation is the lazy load processed. I think we have to
> find out first if the changed render tree with IFC is correct.
> 
> Note that FF and Chrome show the image when loading the reduced test case.
It looks like legacy constructs a 5px tall line for the _empty_ content.

[Rob Buis]

(In reply to zalan from comment #13)
> > Only in the second situation is the lazy load processed. I think we have to
> > find out first if the changed render tree with IFC is correct.
> > 
> > Note that FF and Chrome show the image when loading the reduced test case.
> It looks like legacy constructs a 5px tall line for the _empty_ content.

Good point. I tried this test to mimic the situation:
<!DOCTYPE html>
<meta name="viewport" content="width=device-width,initial-scale=1">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="./resources/intersection-observer-test-utils.js"></script>

<style>
pre, #log {
  position: absolute;
  top: 0;
  left: 200px;
}
#container {
  display: grid;
  height: 0px;
}
</style>

<div id="container">
  <div src="about:blank" style="overflow:hidden">
    <img src="/css/support/60x60-red.png" id='target'></img>
  </div>
</div>

<script>
var entries = [];

runTestCycle(function() {
  var target = document.getElementById('target');
  assert_true(!!target, "target exists");
  var observer = new IntersectionObserver(function(changes) {
    entries = entries.concat(changes)
  });
  observer.observe(target);
  entries = entries.concat(observer.takeRecords());
  assert_equals(entries.length, 0, "No initial notifications.");
  runTestCycle(step0, "First rAF should generate a notification.");
}, "Ensure that a target intersecting a zero-height container generates a notification with intersectionRatio == 0");

function step0() {
  assert_equals(entries.length, 1, "One notification.");
  assert_equals(entries[0].intersectionRatio, 0, "intersectionRatio == 0");
}
</script>

So, given a zero-height container and parent with overflow:hidden, the inner image is expected to not intersect (regardless whether it is lazy loading) by all implementations (all PASS).

With that in mind, with the IFC behaviour the intersection is not supposed to happen so lazy loading will not be triggered (see test case above), with legacy behaviour the intersection is supposed to happen so lazy loading will be triggered.

If IFC behaviour here is what we want/what is specced then the behaviour in this reported bug is as expected.

A workaround I suspect may work in this case is removing the overflow: hidden.

[alan]

Actually the line box is not empty as we are in standards mode here (DOCTYPE). Standards mode puts an invisible strut in every root inline box making the line box contentful. (I should have taken a look at the content and not just the tree dump)

[alan]

Created attachment 463934 [details]
Patch

[Rob Buis]

(In reply to zalan from comment #15)
> Actually the line box is not empty as we are in standards mode here
> (DOCTYPE). Standards mode puts an invisible strut in every root inline box
> making the line box contentful. (I should have taken a look at the content
> and not just the tree dump)

I did not expect that, I was thinking maybe some minimum height was being used internally. Maybe a link to the relevant spec can be put in the commit log?

[EWS]

Committed 257565@main (7e8fe9be9b22): <https://commits.webkit.org/257565@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 463934 [details].

[alan]

(In reply to Rob Buis from comment #17)
> (In reply to zalan from comment #15)
> > Actually the line box is not empty as we are in standards mode here
> > (DOCTYPE). Standards mode puts an invisible strut in every root inline box
> > making the line box contentful. (I should have taken a look at the content
> > and not just the tree dump)
> 
> I did not expect that, I was thinking maybe some minimum height was being
> used internally. Maybe a link to the relevant spec can be put in the commit
> log?
You are right. I assumed it was clear from the commit message that this was about inline layout (and there's only one inline layout spec).
Thanks for looking into it btw.