Bug 1668136 - Set visible the content relevancy of an element with content-visibility:auto if its descendant is called scrollIntoView, r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D186943

dom/base/DOMIntersectionObserver.cpp

@@ -803,8 +803,16 @@ void DOMIntersectionObserver::Update(Document& aDocument,
       }
     }
 
+    // If descendantScrolledIntoView, it means the target is with c-v: auto, and
+    // the content relevancy value has been set to visible before
+    // scrollIntoView. Here, we need to generate entries for them, so that the
+    // content relevancy value could be checked in the callback.
+    const bool temporarilyVisibleForScrolledIntoView =
+        isContentVisibilityObserver == IsContentVisibilityObserver::Yes &&
+        target->TemporarilyVisibleForScrolledIntoViewDescendant();
     // Steps 2.10 - 2.15.
-    if (target->UpdateIntersectionObservation(this, thresholdIndex)) {
+    if (target->UpdateIntersectionObservation(this, thresholdIndex) ||
+        temporarilyVisibleForScrolledIntoView) {
       // See https://github.com/w3c/IntersectionObserver/issues/432 about
       // why we use thresholdIndex > 0 rather than isIntersecting for the
       // entry's isIntersecting value.
@@ -813,6 +821,10 @@ void DOMIntersectionObserver::Update(Document& aDocument,
           output.mIsSimilarOrigin ? Some(output.mRootBounds) : Nothing(),
           output.mTargetRect, output.mIntersectionRect, thresholdIndex > 0,
           intersectionRatio);
+
+      if (temporarilyVisibleForScrolledIntoView) {
+        target->SetTemporarilyVisibleForScrolledIntoViewDescendant(false);
+      }
     }
   }
 }

dom/base/Element.h

@@ -1373,9 +1373,20 @@ class Element : public FragmentOrElement {
     if (auto* slots = GetExistingExtendedDOMSlots()) {
       slots->mContentRelevancy.reset();
       slots->mVisibleForContentVisibility.reset();
+      slots->mTemporarilyVisibleForScrolledIntoViewDescendant = false;
     }
   }
 
+  bool TemporarilyVisibleForScrolledIntoViewDescendant() const {
+    const auto* slots = GetExistingExtendedDOMSlots();
+    return slots && slots->mTemporarilyVisibleForScrolledIntoViewDescendant;
+  }
+
+  void SetTemporarilyVisibleForScrolledIntoViewDescendant(bool aVisible) {
+    ExtendedDOMSlots()->mTemporarilyVisibleForScrolledIntoViewDescendant =
+        aVisible;
+  }
+
   // https://drafts.csswg.org/cssom-view-1/#dom-element-checkvisibility
   MOZ_CAN_RUN_SCRIPT bool CheckVisibility(const CheckVisibilityOptions&);
 

dom/base/FragmentOrElement.h

@@ -242,6 +242,12 @@ class FragmentOrElement : public nsIContent {
      */
     Maybe<bool> mVisibleForContentVisibility;
 
+    /**
+     * Whether content-visibility: auto is temporarily visible for
+     * the purposes of the descendant of scrollIntoView.
+     */
+    bool mTemporarilyVisibleForScrolledIntoViewDescendant = false;
+
     /**
      * Explicitly set attr-elements, see
      * https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#explicitly-set-attr-element

layout/base/PresShell.cpp

@@ -3533,23 +3533,49 @@ nsresult PresShell::ScrollContentIntoView(nsIContent* aContent,
     mContentToScrollTo = nullptr;
   }
 
-  // If the target frame is an ancestor of a `content-visibility: auto`
+  // If the target frame has an ancestor of a `content-visibility: auto`
   // element ensure that it is laid out, so that the boundary rectangle is
   // correct.
+  // Additionally, ensure that all ancestor elements with 'content-visibility:
+  // auto' are set to 'visible'. so that they are laid out as visible before
+  // scrolling, improving the accuracy of the scroll position, especially when
+  // the scroll target is within the overflow area. And here invoking
+  // 'SetTemporarilyVisibleForScrolledIntoViewDescendant' would make the
+  // intersection observer knows that it should generate entries for these
+  // c-v:auto ancestors, so that the content relevancy could be checked again
+  // after scrolling. https://drafts.csswg.org/css-contain-2/#cv-notes
+  bool reflowedForHiddenContent = false;
   if (mContentToScrollTo) {
     if (nsIFrame* frame = mContentToScrollTo->GetPrimaryFrame()) {
-      if (frame->IsHiddenByContentVisibilityOnAnyAncestor(
-              nsIFrame::IncludeContentVisibility::Auto)) {
-        frame->PresShell()->EnsureReflowIfFrameHasHiddenContent(frame);
+      bool hasContentVisibilityAutoAncestor = false;
+      auto* ancestor = frame->GetClosestContentVisibilityAncestor(
+          nsIFrame::IncludeContentVisibility::Auto);
+      while (ancestor) {
+        if (auto* element = Element::FromNodeOrNull(ancestor->GetContent())) {
+          hasContentVisibilityAutoAncestor = true;
+          element->SetTemporarilyVisibleForScrolledIntoViewDescendant(true);
+          element->SetVisibleForContentVisibility(true);
+        }
+        ancestor = ancestor->GetClosestContentVisibilityAncestor(
+            nsIFrame::IncludeContentVisibility::Auto);
+      }
+      if (hasContentVisibilityAutoAncestor) {
+        UpdateHiddenContentInForcedLayout(frame);
+        // TODO: There might be the other already scheduled relevancy updates,
+        // other than caused be scrollIntoView.
+        UpdateContentRelevancyImmediately(ContentRelevancyReason::Visible);
+        reflowedForHiddenContent = ReflowForHiddenContentIfNeeded();
       }
     }
   }
 
-  // Flush layout and attempt to scroll in the process.
-  if (PresShell* presShell = composedDoc->GetPresShell()) {
-    presShell->SetNeedLayoutFlush();
+  if (!reflowedForHiddenContent) {
+    // Flush layout and attempt to scroll in the process.
+    if (PresShell* presShell = composedDoc->GetPresShell()) {
+      presShell->SetNeedLayoutFlush();
+    }
+    composedDoc->FlushPendingNotifications(FlushType::InterruptibleLayout);
   }
-  composedDoc->FlushPendingNotifications(FlushType::InterruptibleLayout);
 
   // If mContentToScrollTo is non-null, that means we interrupted the reflow
   // (or suppressed it altogether because we're suppressing interruptible
@@ -11850,18 +11876,21 @@ bool PresShell::GetZoomableByAPZ() const {
   return mZoomConstraintsClient && mZoomConstraintsClient->GetAllowZoom();
 }
 
-void PresShell::EnsureReflowIfFrameHasHiddenContent(nsIFrame* aFrame) {
+bool PresShell::ReflowForHiddenContentIfNeeded() {
+  if (mHiddenContentInForcedLayout.IsEmpty()) {
+    return false;
+  }
+  mDocument->FlushPendingNotifications(FlushType::Layout);
+  mHiddenContentInForcedLayout.Clear();
+  return true;
+}
+
+void PresShell::UpdateHiddenContentInForcedLayout(nsIFrame* aFrame) {
   if (!aFrame || !aFrame->IsSubtreeDirty() ||
       !StaticPrefs::layout_css_content_visibility_enabled()) {
     return;
   }
 
-  // Flushing notifications below might trigger more layouts, which might,
-  // in turn, trigger layout of other hidden content. We keep a local set
-  // of hidden content we are laying out to handle recursive calls.
-  nsTHashSet<nsIContent*> hiddenContentInForcedLayout;
-
-  MOZ_ASSERT(mHiddenContentInForcedLayout.IsEmpty());
   nsIFrame* topmostFrameWithContentHidden = nullptr;
   for (nsIFrame* cur = aFrame->GetInFlowParent(); cur;
        cur = cur->GetInFlowParent()) {
@@ -11879,9 +11908,13 @@ void PresShell::EnsureReflowIfFrameHasHiddenContent(nsIFrame* aFrame) {
   MOZ_ASSERT(topmostFrameWithContentHidden);
   FrameNeedsReflow(topmostFrameWithContentHidden, IntrinsicDirty::None,
                    NS_FRAME_IS_DIRTY);
-  mDocument->FlushPendingNotifications(FlushType::Layout);
+}
 
-  mHiddenContentInForcedLayout.Clear();
+void PresShell::EnsureReflowIfFrameHasHiddenContent(nsIFrame* aFrame) {
+  MOZ_ASSERT(mHiddenContentInForcedLayout.IsEmpty());
+
+  UpdateHiddenContentInForcedLayout(aFrame);
+  ReflowForHiddenContentIfNeeded();
 }
 
 bool PresShell::IsForcingLayoutForHiddenContent(const nsIFrame* aFrame) const {
@@ -11912,3 +11945,15 @@ void PresShell::ScheduleContentRelevancyUpdate(ContentRelevancyReason aReason) {
     presContext->RefreshDriver()->EnsureContentRelevancyUpdateHappens();
   }
 }
+
+void PresShell::UpdateContentRelevancyImmediately(
+    ContentRelevancyReason aReason) {
+  if (MOZ_UNLIKELY(mIsDestroying)) {
+    return;
+  }
+
+  mContentVisibilityRelevancyToUpdate += aReason;
+
+  SetNeedLayoutFlush();
+  UpdateRelevancyOfContentVisibilityAutoFrames();
+}

layout/base/PresShell.h

@@ -1729,6 +1729,8 @@ class PresShell final : public nsStubDocumentObserver,
 
   bool GetZoomableByAPZ() const;
 
+  bool ReflowForHiddenContentIfNeeded();
+  void UpdateHiddenContentInForcedLayout(nsIFrame*);
   /**
    * If this frame has content hidden via `content-visibilty` that has a pending
    * reflow, force the content to reflow immediately.
@@ -1747,6 +1749,7 @@ class PresShell final : public nsStubDocumentObserver,
 
   void UpdateRelevancyOfContentVisibilityAutoFrames();
   void ScheduleContentRelevancyUpdate(ContentRelevancyReason aReason);
+  void UpdateContentRelevancyImmediately(ContentRelevancyReason aReason);
 
  private:
   ~PresShell();

layout/generic/nsIFrame.cpp

@@ -6924,10 +6924,10 @@ bool nsIFrame::IsHiddenByContentVisibilityOfInFlowParentForLayout() const {
            Style()->IsAnonBox());
 }
 
-bool nsIFrame::IsHiddenByContentVisibilityOnAnyAncestor(
+nsIFrame* nsIFrame::GetClosestContentVisibilityAncestor(
     const EnumSet<IncludeContentVisibility>& aInclude) const {
   if (!StaticPrefs::layout_css_content_visibility_enabled()) {
-    return false;
+    return nullptr;
   }
 
   auto* parent = GetInFlowParent();
@@ -6935,7 +6935,7 @@ bool nsIFrame::IsHiddenByContentVisibilityOnAnyAncestor(
                           parent->HasAnyStateBits(NS_FRAME_OWNS_ANON_BOXES);
   for (nsIFrame* cur = parent; cur; cur = cur->GetInFlowParent()) {
     if (!isAnonymousBlock && cur->HidesContent(aInclude)) {
-      return true;
+      return cur;
     }
 
     // Anonymous boxes are not hidden by the content-visibility of their first
@@ -6944,7 +6944,12 @@ bool nsIFrame::IsHiddenByContentVisibilityOnAnyAncestor(
     isAnonymousBlock = false;
   }
 
-  return false;
+  return nullptr;
+}
+
+bool nsIFrame::IsHiddenByContentVisibilityOnAnyAncestor(
+    const EnumSet<IncludeContentVisibility>& aInclude) const {
+  return !!GetClosestContentVisibilityAncestor(aInclude);
 }
 
 bool nsIFrame::HasSelectionInSubtree() {

layout/generic/nsIFrame.h

@@ -3220,6 +3220,14 @@ class nsIFrame : public nsQueryFrame {
    */
   bool HidesContentForLayout() const;
 
+  /**
+   * returns the closest ancestor with `content-visibility` property.
+   * @param aInclude specifies what kind of `content-visibility` to include.
+   */
+  nsIFrame* GetClosestContentVisibilityAncestor(
+      const mozilla::EnumSet<IncludeContentVisibility>& =
+          IncludeAllContentVisibility()) const;
+
   /**
    * Returns true if this frame is entirely hidden due the `content-visibility`
    * property on an ancestor.

testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-058.html.ini

@@ -1,3 +0,0 @@
-[content-visibility-058.html]
-  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1797467
-  expected: FAIL

testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-064.html.ini

@@ -1,3 +0,0 @@
-[content-visibility-064.html]
-  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1800868
-  expected: FAIL

testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-075.html.ini

@@ -1,2 +1,3 @@
 [content-visibility-075.html]
-  expected: FAIL
+  fuzzy: # Tiny pixels differ around the scrollbar thumb.
+    if (os == "mac"): maxDifference=19;totalPixels=26

testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-076.html.ini

@@ -1,2 +1,3 @@
 [content-visibility-076.html]
-  expected: [FAIL, PASS]
+  fuzzy: # Tiny pixels differ around the scrollbar thumb.
+    if (os == "mac"): maxDifference=19;totalPixels=26

testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html.ini

@@ -0,0 +1,3 @@
+[content-visibility-vs-scrollIntoView-001.html]
+  fuzzy: # Tiny pixels differ around "P".
+    if os == "win": maxDifference=47;totalPixels=2

testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html.ini

@@ -0,0 +1,6 @@
+[content-visibility-vs-scrollIntoView-003.html]
+  expected:
+    if os == "linux": ERROR
+  [ContentVisibilityAutoStateChange fires twice when `scrollIntoView` a descendant of `content-visibility:auto` which is hidden after scrolling]
+    expected:
+      if os == "linux": TIMEOUT

testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001-ref.html

@@ -0,0 +1,41 @@
+<!doctype HTML>
+<html class="reftest-wait">
+<meta charset="utf8">
+<title>Nested CSS Content Visibility: auto + scrollIntoView</title>
+<link rel="author" title="Cathie Chen" href="mailto:cathiechen@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<meta name="assert" content="Test if target scrollIntoView is visible when it is inside a nested content-visibility: auto">
+
+<script src="/common/reftest-wait.js"></script>
+
+<style>
+
+    .child {
+        height: 40000px;
+        position: relative;
+    }
+
+    #target {
+        position: absolute;
+        bottom: 0;
+    }
+
+    .before_target {
+        height: 40000px;
+    }
+</style>
+
+<div id=e1 class="before_target"></div>
+<div id=e2 class="before_target"></div>
+<div id=e3 class="before_target"></div>
+<div id=e4 class="before_target"></div>
+<div id=e5 class=child>
+    <div id=target>PASS</div>
+</div>
+
+<script>
+    window.onload = () => {
+        target.scrollIntoView();
+        requestAnimationFrame(takeScreenshot);
+    }
+</script>

testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html

@@ -0,0 +1,58 @@
+<!doctype HTML>
+<html class="reftest-wait">
+<meta charset="utf8">
+<title>Nested CSS Content Visibility: auto + scrollIntoView</title>
+<link rel="author" title="Cathie Chen" href="mailto:cathiechen@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<link rel="match" href="content-visibility-vs-scrollIntoView-001-ref.html">
+<meta name="assert"
+    content="Test if target scrollIntoView is visible when it is inside a nested content-visibility: auto">
+
+<script src="/common/reftest-wait.js"></script>
+
+<style>
+    .auto {
+        content-visibility: auto;
+        contain-intrinsic-size: auto 1px auto 10000px;
+    }
+
+    .child {
+        height: 40000px;
+        position: relative;
+    }
+
+    #target {
+        position: absolute;
+        bottom: 0;
+    }
+
+    .before_target {
+        height: 40000px;
+    }
+</style>
+
+<div id=e1 class="auto before_target"></div>
+<div id=e2 class="auto before_target"></div>
+<div id=e3 class=auto>
+    <div class=auto>
+        <div class=child></div>
+        <div class=auto>
+            <div class=child></div>
+            <div class=auto>
+                <div class=child>
+                    <div id=target>PASS</div>
+                </div>
+            </div>
+        </div>
+    </div>
+</div>
+
+<script>
+    function runTest() {
+        target.scrollIntoView();
+        // Double rAF to ensure that rendering has "settled".
+        requestAnimationFrame(() => requestAnimationFrame(takeScreenshot));
+    }
+
+    window.onload = () => requestAnimationFrame(() => requestAnimationFrame(runTest));
+</script>

testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002-ref.html

@@ -0,0 +1,59 @@
+<!doctype HTML>
+<html class="reftest-wait">
+<meta charset="utf8">
+<title>CSS Content Visibility: auto + overflow clip + scrollIntoView</title>
+<link rel="author" title="Cathie Chen" href="mailto:cathiechen@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<meta name="assert"
+    content="Test if target scrollIntoView is hidden when it is inside the overflow area of a content-visibility: auto which is not relevent content">
+
+<script src="/common/reftest-wait.js"></script>
+
+<style>
+    .auto {
+        content-visibility: auto;
+        contain-intrinsic-size: auto 1px auto 10000px;
+    }
+
+    .child {
+        height: 40000px;
+        position: relative;
+    }
+
+    #target {
+        position: absolute;
+        bottom: 0;
+    }
+
+    .before_target {
+        height: 40000px;
+    }
+
+    #overflow_clip {
+        overflow: clip;
+        height: 20000px;
+    }
+</style>
+
+<div id=e1 class="auto before_target"></div>
+<div id=e2 class="auto before_target"></div>
+<div id=e3 class="auto">
+    <div id="overflow_clip">
+        <div class=child>
+            <div id=target>PASS</div>
+        </div>
+    </div>
+</div>
+<div id=e4 class=auto>
+    <div class=child></div>
+</div>
+
+
+<script>
+    function runTest() {
+        target.scrollIntoView();
+        requestAnimationFrame(() => requestAnimationFrame(takeScreenshot));
+    }
+
+    window.onload = () => requestAnimationFrame(() => requestAnimationFrame(runTest));
+</script>

testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002.html

@@ -0,0 +1,63 @@
+<!doctype HTML>
+<html class="reftest-wait">
+<meta charset="utf8">
+<title>CSS Content Visibility: auto + overflow clip + scrollIntoView</title>
+<link rel="author" title="Cathie Chen" href="mailto:cathiechen@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<link rel="match" href="content-visibility-vs-scrollIntoView-002-ref.html">
+<meta name="assert"
+    content="content-visibility: auto element not relevent to user should be hidden even after calling scrollIntoView of its descendant">
+
+<script src="/common/reftest-wait.js"></script>
+
+<style>
+    .auto {
+        content-visibility: auto;
+        contain-intrinsic-size: auto 1px auto 10000px;
+    }
+
+    .child {
+        height: 40000px;
+        position: relative;
+    }
+
+    #target {
+        position: absolute;
+        bottom: 0;
+    }
+
+    .before_target {
+        height: 40000px;
+    }
+
+    #overflow_clip {
+        overflow: clip;
+        height: 20000px;
+    }
+</style>
+
+<div id=e1 class="auto before_target"></div>
+<div id=e2 class="auto before_target"></div>
+<div id=e3 class="auto">
+    <div id="overflow_clip">
+        <div class=child>
+            <div id=target>PASS</div>
+        </div>
+    </div>
+</div>
+<div id=e4 class=auto>
+    <div class=child></div>
+</div>
+
+<script>
+    function runTest() {
+        target.scrollIntoView();
+        requestAnimationFrame(() => requestAnimationFrame(() => {
+            // Remove the fixed value of height, so that the computed height would be 40000px.
+            // e3 should be hidden now, "PASS" should not show up.
+            overflow_clip.style.height = "auto";
+            requestAnimationFrame(() => requestAnimationFrame(takeScreenshot));
+        }));
+    }
+    window.onload = () => requestAnimationFrame(() => requestAnimationFrame(runTest));
+</script>

testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html

@@ -0,0 +1,81 @@
+<!doctype HTML>
+<html>
+<meta charset="utf8">
+<title>CSS Content Visibility: auto + overflow clip + scrollIntoView, ContentVisibilityAutoStateChange fires twice</title>
+<link rel="author" title="Cathie Chen" href="mailto:cathiechen@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<meta name="assert"
+    content="If content-visibility: auto element is not relevent to user after calling scrollIntoView of its descendant, contentvisibilityautostatechange twice">
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<style>
+    .auto {
+        content-visibility: auto;
+        contain-intrinsic-size: auto 1px auto 10000px;
+    }
+
+    .child {
+        height: 40000px;
+        position: relative;
+    }
+
+    #target {
+        position: absolute;
+        bottom: 0;
+    }
+
+    .before_target {
+        height: 40000px;
+    }
+
+    #overflow_clip {
+        overflow: clip;
+        height: 20000px;
+    }
+</style>
+
+<div id=e1 class="auto before_target"></div>
+<div id=e2 class="auto before_target"></div>
+<div id=e3 class="auto">
+    <div id="overflow_clip">
+        <div class=child>
+            <div id=target>PASS</div>
+        </div>
+    </div>
+</div>
+<div id=e4 class=auto>
+    <div class=child></div>
+</div>
+
+<script>
+promise_test(t => new Promise(async (resolve, reject) => {
+    await new Promise((waited, _) => {
+        requestAnimationFrame(() => requestAnimationFrame(waited));
+    });
+
+    function waitForEvent() {
+        return new Promise(resolve => e3.addEventListener('contentvisibilityautostatechange', resolve));
+    }
+
+    var eventCounter = 0;
+    function eventHandler(e) {
+        eventCounter++;
+        if (eventCounter == 1) {
+            assert_equals(e.skipped, false, "the first event should be generated by visible");
+        } else if (eventCounter == 2) {
+            assert_equals(e.skipped, true, "the second event should be generated by hidden");
+        }
+    }
+
+    e3.addEventListener("contentvisibilityautostatechange", eventHandler);
+    target.scrollIntoView();
+    await waitForEvent();
+    await waitForEvent();
+    requestAnimationFrame(() => requestAnimationFrame(() => {
+        assert_equals(eventCounter, 2, "There should be two contentvisibilityautostatechange events.");
+        resolve();
+    }));
+}), "ContentVisibilityAutoStateChange fires twice when `scrollIntoView` a descendant of `content-visibility:auto` which is hidden after scrolling");
+</script>