diff --git a/dom/base/DOMIntersectionObserver.cpp b/dom/base/DOMIntersectionObserver.cpp index 2b805cda8b3ad..40cc8dae516a0 100644 --- a/dom/base/DOMIntersectionObserver.cpp +++ b/dom/base/DOMIntersectionObserver.cpp @@ -499,10 +499,9 @@ void DOMIntersectionObserver::Update(Document* aDocument, if (mRoot && mRoot->IsElement()) { if ((rootFrame = mRoot->AsElement()->GetPrimaryFrame())) { nsRect rootRectRelativeToRootFrame; - if (rootFrame->IsScrollFrame()) { + if (nsIScrollableFrame* scrollFrame = do_QueryFrame(rootFrame)) { // rootRectRelativeToRootFrame should be the content rect of rootFrame, // not including the scrollbars. - nsIScrollableFrame* scrollFrame = do_QueryFrame(rootFrame); rootRectRelativeToRootFrame = scrollFrame->GetScrollPortRect(); } else { // rootRectRelativeToRootFrame should be the border rect of rootFrame. @@ -532,9 +531,11 @@ void DOMIntersectionObserver::Update(Document* aDocument, // there's any OOP iframe in between `rootDocument` and `aDocument`, to // handle the OOP iframe positions. if (PresShell* presShell = rootDocument->GetPresShell()) { - rootFrame = presShell->GetRootScrollFrame(); - if (rootFrame) { - nsIScrollableFrame* scrollFrame = do_QueryFrame(rootFrame); + rootFrame = presShell->GetRootFrame(); + // We use the root scrollable frame's scroll port to account the + // scrollbars in rootRect, if needed. + if (nsIScrollableFrame* scrollFrame = + presShell->GetRootScrollFrameAsScrollable()) { rootRect = scrollFrame->GetScrollPortRect(); } } @@ -563,33 +564,53 @@ void DOMIntersectionObserver::Update(Document* aDocument, // processed in the same order that observe() was called on each target: for (Element* target : mObservationTargets) { nsIFrame* targetFrame = target->GetPrimaryFrame(); - // 2.2. If the intersection root is not the implicit root, and target is not - // in the same Document as the intersection root, skip further processing - // for target. - if (mRoot && mRoot->OwnerDoc() != target->OwnerDoc()) { - continue; - } - - nsRect rootBounds; - if (rootFrame && targetFrame) { - // FIXME(emilio): Why only if there are frames? - rootBounds = rootRect; - } - BrowsingContextOrigin origin = SimilarOrigin(*target, root); - if (origin == BrowsingContextOrigin::Similar) { - rootBounds.Inflate(rootMargin); - } Maybe intersectionRect; nsRect targetRect; - if (targetFrame && rootFrame) { + nsRect rootBounds; + + const bool canComputeIntersection = [&] { + if (!targetFrame || !rootFrame) { + return false; + } + // 2.1. If the intersection root is not the implicit root and target is // not a descendant of the intersection root in the containing block // chain, skip further processing for target. - if (mRoot && !nsLayoutUtils::IsProperAncestorFrameCrossDoc(rootFrame, - targetFrame)) { - continue; + // + // NOTE(emilio): We don't just "skip further processing" because that + // violates the invariant that there's at least one observation for a + // target (though that is also violated by 2.2), but it also causes + // different behavior when `target` is `display: none`, or not, which is + // really really odd, see: + // https://github.com/w3c/IntersectionObserver/issues/457 + // + // NOTE(emilio): We also do this if target is the implicit root, pending + // clarification in + // https://github.com/w3c/IntersectionObserver/issues/456. + if (!nsLayoutUtils::IsAncestorFrameCrossDoc(rootFrame, targetFrame)) { + return false; + } + + // 2.2. If the intersection root is not the implicit root, and target is + // not in the same Document as the intersection root, skip further + // processing for target. + // + // NOTE(emilio): We don't just "skip further processing", because that + // doesn't match reality and other browsers, see + // https://github.com/w3c/IntersectionObserver/issues/457. + if (mRoot && mRoot->OwnerDoc() != target->OwnerDoc()) { + return false; + } + + return true; + }(); + + if (canComputeIntersection) { + rootBounds = rootRect; + if (origin == BrowsingContextOrigin::Similar) { + rootBounds.Inflate(rootMargin); } // 2.3. Let targetRect be a DOMRectReadOnly obtained by running the diff --git a/dom/base/test/test_intersectionobservers.html b/dom/base/test/test_intersectionobservers.html index 45a266fbb43eb..d228dfc9440c2 100644 --- a/dom/base/test/test_intersectionobservers.html +++ b/dom/base/test/test_intersectionobservers.html @@ -333,20 +333,6 @@ }); - it('does not trigger if target is not a descendant of the intersection root in the containing block chain', - function(done) { - - var spy = sinon.spy(); - io = new IntersectionObserver(spy, {root: parentEl}); - - parentEl.style.position = 'static'; - io.observe(targetEl2); - callDelayed(function() { - expect(spy.callCount).to.be(0); - done(); - }); - }); - it('triggers if target or root becomes invisible', function(done) { @@ -929,25 +915,6 @@ describe('observe subframe', function () { - it('should not trigger if target and root are not in the same document', - function(done) { - - var spy = sinon.spy(); - io = new IntersectionObserver(spy, {root: rootEl}); - - targetEl4.onload = function () { - targetEl5 = targetEl4.contentDocument.getElementById('target5'); - io.observe(targetEl5); - callDelayed(function() { - expect(spy.callCount).to.be(0); - done(); - }); - } - - targetEl4.src = "intersectionobserver_iframe.html"; - - }); - it('boundingClientRect matches target.getBoundingClientRect() for an element inside an iframe', function(done) { diff --git a/testing/web-platform/meta/intersection-observer/target-in-different-window.html.ini b/testing/web-platform/meta/intersection-observer/target-in-different-window.html.ini index 53061e4548a85..38477059ad471 100644 --- a/testing/web-platform/meta/intersection-observer/target-in-different-window.html.ini +++ b/testing/web-platform/meta/intersection-observer/target-in-different-window.html.ini @@ -1,2 +1,4 @@ [target-in-different-window.html] - disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1497420 + bug: https://github.com/w3c/IntersectionObserver/issues/456 + [IntersectionObserver with target in a different window.] + expected: FAIL diff --git a/testing/web-platform/tests/intersection-observer/explicit-root-different-document.tentative.html b/testing/web-platform/tests/intersection-observer/explicit-root-different-document.tentative.html new file mode 100644 index 0000000000000..15c5e620d5bea --- /dev/null +++ b/testing/web-platform/tests/intersection-observer/explicit-root-different-document.tentative.html @@ -0,0 +1,27 @@ + + + + + + +
+ diff --git a/testing/web-platform/tests/intersection-observer/not-in-containing-block-chain.tentative.html b/testing/web-platform/tests/intersection-observer/not-in-containing-block-chain.tentative.html new file mode 100644 index 0000000000000..4490d0b631ec2 --- /dev/null +++ b/testing/web-platform/tests/intersection-observer/not-in-containing-block-chain.tentative.html @@ -0,0 +1,25 @@ + + + + + + +
+
+ diff --git a/testing/web-platform/tests/intersection-observer/target-in-different-window.html b/testing/web-platform/tests/intersection-observer/target-in-different-window.html index 645b7ec1908cc..fcf5ba94372be 100644 --- a/testing/web-platform/tests/intersection-observer/target-in-different-window.html +++ b/testing/web-platform/tests/intersection-observer/target-in-different-window.html @@ -1,5 +1,9 @@ +