Backed out changeset 6e5782df6da1 (bug 1668136) for causing wpt failures on content-visibility-vs-scrollIntoView-003.html CLOSED TREE

Author: Norisz Fay

dom/base/DOMIntersectionObserver.cpp

@@ -803,16 +803,8 @@ 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) ||
-        temporarilyVisibleForScrolledIntoView) {
+    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.
@@ -821,10 +813,6 @@ 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,20 +1373,9 @@ 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,12 +242,6 @@ 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,49 +3533,23 @@ nsresult PresShell::ScrollContentIntoView(nsIContent* aContent,
     mContentToScrollTo = nullptr;
   }
 
-  // If the target frame has an ancestor of a `content-visibility: auto`
+  // If the target frame is 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()) {
-      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();
+      if (frame->IsHiddenByContentVisibilityOnAnyAncestor(
+              nsIFrame::IncludeContentVisibility::Auto)) {
+        frame->PresShell()->EnsureReflowIfFrameHasHiddenContent(frame);
       }
     }
   }
 
-  if (!reflowedForHiddenContent) {
-    // Flush layout and attempt to scroll in the process.
-    if (PresShell* presShell = composedDoc->GetPresShell()) {
-      presShell->SetNeedLayoutFlush();
-    }
-    composedDoc->FlushPendingNotifications(FlushType::InterruptibleLayout);
+  // Flush layout and attempt to scroll in the process.
+  if (PresShell* presShell = composedDoc->GetPresShell()) {
+    presShell->SetNeedLayoutFlush();
   }
+  composedDoc->FlushPendingNotifications(FlushType::InterruptibleLayout);
 
   // If mContentToScrollTo is non-null, that means we interrupted the reflow
   // (or suppressed it altogether because we're suppressing interruptible
@@ -11876,21 +11850,18 @@ bool PresShell::GetZoomableByAPZ() const {
   return mZoomConstraintsClient && mZoomConstraintsClient->GetAllowZoom();
 }
 
-bool PresShell::ReflowForHiddenContentIfNeeded() {
-  if (mHiddenContentInForcedLayout.IsEmpty()) {
-    return false;
-  }
-  mDocument->FlushPendingNotifications(FlushType::Layout);
-  mHiddenContentInForcedLayout.Clear();
-  return true;
-}
-
-void PresShell::UpdateHiddenContentInForcedLayout(nsIFrame* aFrame) {
+void PresShell::EnsureReflowIfFrameHasHiddenContent(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()) {
@@ -11908,13 +11879,9 @@ void PresShell::UpdateHiddenContentInForcedLayout(nsIFrame* aFrame) {
   MOZ_ASSERT(topmostFrameWithContentHidden);
   FrameNeedsReflow(topmostFrameWithContentHidden, IntrinsicDirty::None,
                    NS_FRAME_IS_DIRTY);
-}
-
-void PresShell::EnsureReflowIfFrameHasHiddenContent(nsIFrame* aFrame) {
-  MOZ_ASSERT(mHiddenContentInForcedLayout.IsEmpty());
+  mDocument->FlushPendingNotifications(FlushType::Layout);
 
-  UpdateHiddenContentInForcedLayout(aFrame);
-  ReflowForHiddenContentIfNeeded();
+  mHiddenContentInForcedLayout.Clear();
 }
 
 bool PresShell::IsForcingLayoutForHiddenContent(const nsIFrame* aFrame) const {
@@ -11945,15 +11912,3 @@ 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,8 +1729,6 @@ 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.
@@ -1749,7 +1747,6 @@ class PresShell final : public nsStubDocumentObserver,
 
   void UpdateRelevancyOfContentVisibilityAutoFrames();
   void ScheduleContentRelevancyUpdate(ContentRelevancyReason aReason);
-  void UpdateContentRelevancyImmediately(ContentRelevancyReason aReason);
 
  private:
   ~PresShell();

layout/generic/nsIFrame.cpp

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

layout/generic/nsIFrame.h

@@ -3220,14 +3220,6 @@ 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

@@ -0,0 +1,3 @@
+[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

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

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

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