Bug 198784 - IntersectionObserver rootMargin detection fails when `root` is an element
Summary: IntersectionObserver rootMargin detection fails when `root` is an element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ali Juma
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2019-06-11 23:16 PDT by EricD
Modified: 2019-06-17 06:02 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.58 KB, patch)
2019-06-13 13:01 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Patch (8.58 KB, patch)
2019-06-13 13:06 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Patch for landing (8.67 KB, patch)
2019-06-13 14:01 PDT, Ali Juma
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description EricD 2019-06-11 23:16:19 PDT
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.
Comment 1 Radar WebKit Bug Importer 2019-06-12 01:40:17 PDT
<rdar://problem/51659139>
Comment 2 EricD 2019-06-12 03:41:11 PDT
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.
Comment 3 Ali Juma 2019-06-13 11:25:57 PDT
(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.
Comment 4 Ali Juma 2019-06-13 13:01:08 PDT
Created attachment 372075 [details]
Patch
Comment 5 Ali Juma 2019-06-13 13:06:45 PDT
Created attachment 372076 [details]
Patch
Comment 6 Ali Juma 2019-06-13 13:07:24 PDT
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/17323
Comment 7 Simon Fraser (smfr) 2019-06-13 13:23:59 PDT
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 });
Comment 8 Ali Juma 2019-06-13 14:01:45 PDT
Created attachment 372078 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2019-06-14 06:05:19 PDT
Comment on attachment 372078 [details]
Patch for landing

Clearing flags on attachment: 372078

Committed r246432: <https://trac.webkit.org/changeset/246432>
Comment 10 WebKit Commit Bot 2019-06-14 06:05:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 EricD 2019-06-14 15:32:37 PDT
That's super cool. Thanks for fixing. How long do patches like this take to show up in a production release of a browser?
Comment 12 Ali Juma 2019-06-17 06:02:29 PDT
(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.