IntersectionObserver rootMargin detection fails when `root` is an element

[EricD]

Effected versions: Safari 12.1.1, iOS Safari 12.3.1, Safari Technology Preview (Safari 13.0, WebKit 14608.1.25.2), and iOS Chrome 75.0.3770.70. Works however in desktop chrome Version 74.0.3729.169.

rootMargin detection does not work when the root is an element. Instead isIntersecting is only true when it is absolutely in the viewport.

basic example.

    var scrollableContainer = document.querySelector("#scrollableContainer");

    var observer = new IntersectionObserver(
      entries => {
        entries.forEach(entry => {
          if (entry.isIntersecting) {
            entry.target.style.background = "green";
            console.log("is inviewport");
            this.set("viewportEntered", true);
          } else {
            entry.target.style.background = "red";
            this.set("viewportEntered", false);
            console.log("is outside viewport");
          }
        });
      },
      {
        root: scrollableContainer,
        threshold: [0, 0.5, 1],
        rootMargin: "500px"
      }
    );

    observer.observe(this.element);

If you want a fully reproducable web demo, let me know.

[Radar WebKit Bug Importer]

<rdar://problem/51659139>

[EricD]

Fully reproducible demo here:
https://edeis53.github.io/inviewport/?version=28e1794

-Step#1 - Click on "plain intersection-observer with element" menu link.
-NOTE: This is stripped down reproducable demo that contains the minimal amount of JS to reproduce. The other tests in the demo use a web component that wraps intersectionObserver with additional logic. However, all the examples work and show that rootMargin works when detecting using window, but not when using an element.
-Step#2 - Open the console and observe which element is reporting intersection of the viewport.
-Step#3 - Repeat in iOS Chrome and MacOS Desktop Chrome.

The demo performs the following JS:
    var scrollableContainer = document.querySelector("#scrollableContainer");

    var observer = new IntersectionObserver(
      entries => {
        entries.forEach(entry => {
          if (entry.isIntersecting) {
            entry.target.style.background = "green";
            console.log("is inviewport", this.number);
            this.set("viewportEntered", true);
          } else {
            entry.target.style.background = "red";
            this.set("viewportEntered", false);
            console.log("is outside viewport", this.number);
          }
        });
      },
      {
        root: scrollableContainer,
        threshold: [0, 0.5, 1],
        rootMargin: "500px"
      }
    );

    observer.observe(this.element);

In the demo, you will see elements with their respective index number and viewport status displayed in their html element for reference, as they enter the viewport, they will log their entry/exit. 

For example, in iOS Chrome, you will see element#6 entering the viewport on your screen and at the same time element#6 logs it's intersecting to the console, rootMargin is not working in that instance. However, if you try via Chrome Desktop, when element #6 is entering the viewport on your screen, element #9 or #10 will log to console that it is intersecting because rootMargin detection is working properly and they are the next element that is 500px offscreen and entering.

[Ali Juma]

(In reply to EricD from comment #2)
> Fully reproducible demo here:
> https://edeis53.github.io/inviewport/?version=28e1794

Thanks for the detailed report and test case!

The problem is we were applying the root's overflow clip before intersecting with the expanded root, so the target was treated as clipped out.

The web platform test for rootMargin only deals with the case where the root is the viewport, so we didn't catch this earlier.

I'll upload a patch and add a web platform test for this.

[Ali Juma]

Created attachment 372075 [details]
Patch

[Ali Juma]

Created attachment 372076 [details]
Patch

[Ali Juma]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/17323

[Simon Fraser (smfr)]

Comment on attachment 372076 [details]
Patch

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

> Source/WebCore/rendering/RenderObject.cpp:990
>      VisibleRectContext context;
> +    context.m_options.add(VisibleRectContextOption::ApplyContainerClip);

How about:
VisibleRectContext context(false, false, { VisibleRectContextOption::ApplyContainerClip });

[Ali Juma]

Created attachment 372078 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 372078 [details]
Patch for landing

Clearing flags on attachment: 372078

Committed r246432: <https://trac.webkit.org/changeset/246432>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[EricD]

That's super cool. Thanks for fixing. How long do patches like this take to show up in a production release of a browser?

[Ali Juma]

(In reply to EricD from comment #11)
> That's super cool. Thanks for fixing. How long do patches like this take to
> show up in a production release of a browser?

On iOS, all browsers use the version of WebKit that ships with the OS itself, so this fix will show up as part of an OS update rather than a browser release. It typically takes a few months from a patch landing to showing up in a release.