Skip to content

Commit c6bcab8

Browse files
committed
Bug 1611204 - Fix IntersectionObserverEntry.isIntersecting to match other browsers. r=mstange
Note that no browser matches the spec (see w3c/IntersectionObserver#432), but that our behavior is reasonably close to them. So do this to match them. Differential Revision: https://phabricator.services.mozilla.com/D76603
1 parent 6131e9f commit c6bcab8

File tree

4 files changed

+17
-13
lines changed

4 files changed

+17
-13
lines changed

dom/base/DOMIntersectionObserver.cpp

+11-5
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,9 @@ void DOMIntersectionObserver::Update(Document* aDocument,
618618
// length of observer.thresholds if intersectionRatio is greater than or
619619
// equal to the last entry in observer.thresholds.
620620
int32_t thresholdIndex = -1;
621-
// FIXME(emilio): Why the isIntersecting check?
621+
622+
// If not intersecting, we can just shortcut, as we know that the thresholds
623+
// are always between 0 and 1.
622624
if (isIntersecting) {
623625
thresholdIndex = mThresholds.IndexOfFirstElementGt(intersectionRatio);
624626
if (thresholdIndex == 0) {
@@ -628,26 +630,30 @@ void DOMIntersectionObserver::Update(Document* aDocument,
628630
// neither Chrome nor the WPT tests expect this behavior, so treat these
629631
// two cases as one.
630632
//
631-
// FIXME(emilio): Looks like a good candidate for a spec issue.
633+
// See https://github.com/w3c/IntersectionObserver/issues/432 about
634+
// this.
632635
thresholdIndex = -1;
633636
}
634637
}
635638

636639
// Steps 2.10 - 2.15.
637640
if (target->UpdateIntersectionObservation(this, thresholdIndex)) {
641+
// See https://github.com/w3c/IntersectionObserver/issues/432 about
642+
// why we use thresholdIndex > 0 rather than isIntersecting for the
643+
// entry's isIntersecting value.
638644
QueueIntersectionObserverEntry(
639645
target, time,
640646
origin == BrowsingContextOrigin::Similar ? Some(rootBounds)
641647
: Nothing(),
642-
targetRect, intersectionRect, intersectionRatio);
648+
targetRect, intersectionRect, thresholdIndex > 0, intersectionRatio);
643649
}
644650
}
645651
}
646652

647653
void DOMIntersectionObserver::QueueIntersectionObserverEntry(
648654
Element* aTarget, DOMHighResTimeStamp time, const Maybe<nsRect>& aRootRect,
649655
const nsRect& aTargetRect, const Maybe<nsRect>& aIntersectionRect,
650-
double aIntersectionRatio) {
656+
bool aIsIntersecting, double aIntersectionRatio) {
651657
RefPtr<DOMRect> rootBounds;
652658
if (aRootRect.isSome()) {
653659
rootBounds = new DOMRect(this);
@@ -661,7 +667,7 @@ void DOMIntersectionObserver::QueueIntersectionObserverEntry(
661667
}
662668
RefPtr<DOMIntersectionObserverEntry> entry = new DOMIntersectionObserverEntry(
663669
this, time, rootBounds.forget(), boundingClientRect.forget(),
664-
intersectionRect.forget(), aIntersectionRect.isSome(), aTarget,
670+
intersectionRect.forget(), aIsIntersecting, aTarget,
665671
aIntersectionRatio);
666672
mQueuedEntries.AppendElement(entry.forget());
667673
}

dom/base/DOMIntersectionObserver.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ class DOMIntersectionObserverEntry final : public nsISupports,
3131
double aIntersectionRatio)
3232
: mOwner(aOwner),
3333
mTime(aTime),
34-
mRootBounds(aRootBounds),
35-
mBoundingClientRect(aBoundingClientRect),
36-
mIntersectionRect(aIntersectionRect),
34+
mRootBounds(std::move(aRootBounds)),
35+
mBoundingClientRect(std::move(aBoundingClientRect)),
36+
mIntersectionRect(std::move(aIntersectionRect)),
3737
mIsIntersecting(aIsIntersecting),
3838
mTarget(aTarget),
3939
mIntersectionRatio(aIntersectionRatio) {}
@@ -138,6 +138,7 @@ class DOMIntersectionObserver final : public nsISupports,
138138
const Maybe<nsRect>& aRootRect,
139139
const nsRect& aTargetRect,
140140
const Maybe<nsRect>& aIntersectionRect,
141+
bool aIsIntersecting,
141142
double aIntersectionRatio);
142143

143144
nsCOMPtr<nsPIDOMWindowInner> mOwner;

testing/web-platform/meta/intersection-observer/initial-observation-with-threshold.html.ini

-4
This file was deleted.

testing/web-platform/tests/intersection-observer/isIntersecting-threshold.html

+2-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
assert_equals(entries.length, 2);
4444
assert_true(entries[1].intersectionRatio >= 0.5 &&
4545
entries[1].intersectionRatio < 1);
46-
assert_equals(entries[1].isIntersecting, true);
46+
// See https://github.com/w3c/IntersectionObserver/issues/432
47+
assert_equals(entries[1].isIntersecting, false);
4748
scroller.scrollTop = 100;
4849
}
4950

0 commit comments

Comments
 (0)