Bug 1611204 - Fix IntersectionObserverEntry.isIntersecting to match other browsers. r=mstange

Note that no browser matches the spec (see
https://github.com/w3c/IntersectionObserver/issues/432), but that our
behavior is reasonably close to them. So do this to match them.

Differential Revision: https://phabricator.services.mozilla.com/D76603
This commit is contained in:
Emilio Cobos Álvarez 2020-05-28 10:38:51 +00:00
parent 6131e9fa5c
commit c6bcab8a5c
4 changed files with 17 additions and 13 deletions

View File

@ -618,7 +618,9 @@ void DOMIntersectionObserver::Update(Document* aDocument,
// length of observer.thresholds if intersectionRatio is greater than or // length of observer.thresholds if intersectionRatio is greater than or
// equal to the last entry in observer.thresholds. // equal to the last entry in observer.thresholds.
int32_t thresholdIndex = -1; int32_t thresholdIndex = -1;
// FIXME(emilio): Why the isIntersecting check?
// If not intersecting, we can just shortcut, as we know that the thresholds
// are always between 0 and 1.
if (isIntersecting) { if (isIntersecting) {
thresholdIndex = mThresholds.IndexOfFirstElementGt(intersectionRatio); thresholdIndex = mThresholds.IndexOfFirstElementGt(intersectionRatio);
if (thresholdIndex == 0) { if (thresholdIndex == 0) {
@ -628,18 +630,22 @@ void DOMIntersectionObserver::Update(Document* aDocument,
// neither Chrome nor the WPT tests expect this behavior, so treat these // neither Chrome nor the WPT tests expect this behavior, so treat these
// two cases as one. // two cases as one.
// //
// FIXME(emilio): Looks like a good candidate for a spec issue. // See https://github.com/w3c/IntersectionObserver/issues/432 about
// this.
thresholdIndex = -1; thresholdIndex = -1;
} }
} }
// Steps 2.10 - 2.15. // Steps 2.10 - 2.15.
if (target->UpdateIntersectionObservation(this, thresholdIndex)) { if (target->UpdateIntersectionObservation(this, thresholdIndex)) {
// See https://github.com/w3c/IntersectionObserver/issues/432 about
// why we use thresholdIndex > 0 rather than isIntersecting for the
// entry's isIntersecting value.
QueueIntersectionObserverEntry( QueueIntersectionObserverEntry(
target, time, target, time,
origin == BrowsingContextOrigin::Similar ? Some(rootBounds) origin == BrowsingContextOrigin::Similar ? Some(rootBounds)
: Nothing(), : Nothing(),
targetRect, intersectionRect, intersectionRatio); targetRect, intersectionRect, thresholdIndex > 0, intersectionRatio);
} }
} }
} }
@ -647,7 +653,7 @@ void DOMIntersectionObserver::Update(Document* aDocument,
void DOMIntersectionObserver::QueueIntersectionObserverEntry( void DOMIntersectionObserver::QueueIntersectionObserverEntry(
Element* aTarget, DOMHighResTimeStamp time, const Maybe<nsRect>& aRootRect, Element* aTarget, DOMHighResTimeStamp time, const Maybe<nsRect>& aRootRect,
const nsRect& aTargetRect, const Maybe<nsRect>& aIntersectionRect, const nsRect& aTargetRect, const Maybe<nsRect>& aIntersectionRect,
double aIntersectionRatio) { bool aIsIntersecting, double aIntersectionRatio) {
RefPtr<DOMRect> rootBounds; RefPtr<DOMRect> rootBounds;
if (aRootRect.isSome()) { if (aRootRect.isSome()) {
rootBounds = new DOMRect(this); rootBounds = new DOMRect(this);
@ -661,7 +667,7 @@ void DOMIntersectionObserver::QueueIntersectionObserverEntry(
} }
RefPtr<DOMIntersectionObserverEntry> entry = new DOMIntersectionObserverEntry( RefPtr<DOMIntersectionObserverEntry> entry = new DOMIntersectionObserverEntry(
this, time, rootBounds.forget(), boundingClientRect.forget(), this, time, rootBounds.forget(), boundingClientRect.forget(),
intersectionRect.forget(), aIntersectionRect.isSome(), aTarget, intersectionRect.forget(), aIsIntersecting, aTarget,
aIntersectionRatio); aIntersectionRatio);
mQueuedEntries.AppendElement(entry.forget()); mQueuedEntries.AppendElement(entry.forget());
} }

View File

@ -31,9 +31,9 @@ class DOMIntersectionObserverEntry final : public nsISupports,
double aIntersectionRatio) double aIntersectionRatio)
: mOwner(aOwner), : mOwner(aOwner),
mTime(aTime), mTime(aTime),
mRootBounds(aRootBounds), mRootBounds(std::move(aRootBounds)),
mBoundingClientRect(aBoundingClientRect), mBoundingClientRect(std::move(aBoundingClientRect)),
mIntersectionRect(aIntersectionRect), mIntersectionRect(std::move(aIntersectionRect)),
mIsIntersecting(aIsIntersecting), mIsIntersecting(aIsIntersecting),
mTarget(aTarget), mTarget(aTarget),
mIntersectionRatio(aIntersectionRatio) {} mIntersectionRatio(aIntersectionRatio) {}
@ -138,6 +138,7 @@ class DOMIntersectionObserver final : public nsISupports,
const Maybe<nsRect>& aRootRect, const Maybe<nsRect>& aRootRect,
const nsRect& aTargetRect, const nsRect& aTargetRect,
const Maybe<nsRect>& aIntersectionRect, const Maybe<nsRect>& aIntersectionRect,
bool aIsIntersecting,
double aIntersectionRatio); double aIntersectionRatio);
nsCOMPtr<nsPIDOMWindowInner> mOwner; nsCOMPtr<nsPIDOMWindowInner> mOwner;

View File

@ -1,4 +0,0 @@
[initial-observation-with-threshold.html]
[First rAF]
expected: FAIL

View File

@ -43,7 +43,8 @@ function step3() {
assert_equals(entries.length, 2); assert_equals(entries.length, 2);
assert_true(entries[1].intersectionRatio >= 0.5 && assert_true(entries[1].intersectionRatio >= 0.5 &&
entries[1].intersectionRatio < 1); entries[1].intersectionRatio < 1);
assert_equals(entries[1].isIntersecting, true); // See https://github.com/w3c/IntersectionObserver/issues/432
assert_equals(entries[1].isIntersecting, false);
scroller.scrollTop = 100; scroller.scrollTop = 100;
} }