From 37cbcfe9beed9a973050263341c408b252e3fc49 Mon Sep 17 00:00:00 2001 From: Norisz Fay Date: Wed, 1 Nov 2023 18:02:54 +0200 Subject: [PATCH] Backed out changeset 6e5782df6da1 (bug 1668136) for causing wpt failures on content-visibility-vs-scrollIntoView-003.html CLOSED TREE --- dom/base/DOMIntersectionObserver.cpp | 14 +--- dom/base/Element.h | 11 --- dom/base/FragmentOrElement.h | 6 -- layout/base/PresShell.cpp | 79 ++++-------------- layout/base/PresShell.h | 3 - layout/generic/nsIFrame.cpp | 13 +-- layout/generic/nsIFrame.h | 8 -- .../content-visibility-058.html.ini | 3 + .../content-visibility-064.html.ini | 3 + .../content-visibility-075.html.ini | 3 +- .../content-visibility-076.html.ini | 3 +- ...-visibility-vs-scrollIntoView-001.html.ini | 3 - ...-visibility-vs-scrollIntoView-003.html.ini | 6 -- ...-visibility-vs-scrollIntoView-001-ref.html | 41 ---------- ...tent-visibility-vs-scrollIntoView-001.html | 58 ------------- ...-visibility-vs-scrollIntoView-002-ref.html | 59 -------------- ...tent-visibility-vs-scrollIntoView-002.html | 63 --------------- ...tent-visibility-vs-scrollIntoView-003.html | 81 ------------------- 18 files changed, 30 insertions(+), 427 deletions(-) create mode 100644 testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-058.html.ini create mode 100644 testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-064.html.ini delete mode 100644 testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html.ini delete mode 100644 testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html.ini delete mode 100644 testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001-ref.html delete mode 100644 testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html delete mode 100644 testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002-ref.html delete mode 100644 testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002.html delete mode 100644 testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html diff --git a/dom/base/DOMIntersectionObserver.cpp b/dom/base/DOMIntersectionObserver.cpp index 592b9bef7798..76411b4b6f78 100644 --- a/dom/base/DOMIntersectionObserver.cpp +++ b/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); - } } } } diff --git a/dom/base/Element.h b/dom/base/Element.h index 762aef16e93a..b71c363f224f 100644 --- a/dom/base/Element.h +++ b/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&); diff --git a/dom/base/FragmentOrElement.h b/dom/base/FragmentOrElement.h index 56fd2796e61b..2eace861e598 100644 --- a/dom/base/FragmentOrElement.h +++ b/dom/base/FragmentOrElement.h @@ -242,12 +242,6 @@ class FragmentOrElement : public nsIContent { */ Maybe 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 diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 7d85ac5ed90f..b02283dea6f9 100644 --- a/layout/base/PresShell.cpp +++ b/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 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); -} + mDocument->FlushPendingNotifications(FlushType::Layout); -void PresShell::EnsureReflowIfFrameHasHiddenContent(nsIFrame* aFrame) { - MOZ_ASSERT(mHiddenContentInForcedLayout.IsEmpty()); - - 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(); -} diff --git a/layout/base/PresShell.h b/layout/base/PresShell.h index c590d0b67d51..cc733aa7ea94 100644 --- a/layout/base/PresShell.h +++ b/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(); diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp index 9e31dbfaa65b..3a336b25278a 100644 --- a/layout/generic/nsIFrame.cpp +++ b/layout/generic/nsIFrame.cpp @@ -6924,10 +6924,10 @@ bool nsIFrame::IsHiddenByContentVisibilityOfInFlowParentForLayout() const { Style()->IsAnonBox()); } -nsIFrame* nsIFrame::GetClosestContentVisibilityAncestor( +bool nsIFrame::IsHiddenByContentVisibilityOnAnyAncestor( const EnumSet& aInclude) const { if (!StaticPrefs::layout_css_content_visibility_enabled()) { - return nullptr; + return false; } auto* parent = GetInFlowParent(); @@ -6935,7 +6935,7 @@ nsIFrame* nsIFrame::GetClosestContentVisibilityAncestor( 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& aInclude) const { - return !!GetClosestContentVisibilityAncestor(aInclude); + return false; } bool nsIFrame::HasSelectionInSubtree() { diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index cc6c5472f498..80c3504e1af3 100644 --- a/layout/generic/nsIFrame.h +++ b/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& = - IncludeAllContentVisibility()) const; - /** * Returns true if this frame is entirely hidden due the `content-visibility` * property on an ancestor. diff --git a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-058.html.ini b/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-058.html.ini new file mode 100644 index 000000000000..0f634857b51e --- /dev/null +++ b/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 diff --git a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-064.html.ini b/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-064.html.ini new file mode 100644 index 000000000000..0100d6fd864a --- /dev/null +++ b/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 diff --git a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-075.html.ini b/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-075.html.ini index 64766648989a..ee6136cbc0d1 100644 --- a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-075.html.ini +++ b/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 diff --git a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-076.html.ini b/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-076.html.ini index c13637cb9a21..cb9794ec79e5 100644 --- a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-076.html.ini +++ b/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] diff --git a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html.ini b/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html.ini deleted file mode 100644 index 9bb70da5aae3..000000000000 --- a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[content-visibility-vs-scrollIntoView-001.html] - fuzzy: # Tiny pixels differ around "P". - if os == "win": maxDifference=47;totalPixels=2 diff --git a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html.ini b/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html.ini deleted file mode 100644 index 05e50bd80e9c..000000000000 --- a/testing/web-platform/meta/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[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 diff --git a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001-ref.html b/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001-ref.html deleted file mode 100644 index 897463844aa4..000000000000 --- a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001-ref.html +++ /dev/null @@ -1,41 +0,0 @@ - - - -Nested CSS Content Visibility: auto + scrollIntoView - - - - - - - - -
-
-
-
-
-
PASS
-
- - diff --git a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html b/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html deleted file mode 100644 index 0f874e660753..000000000000 --- a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-001.html +++ /dev/null @@ -1,58 +0,0 @@ - - - -Nested CSS Content Visibility: auto + scrollIntoView - - - - - - - - - -
-
-
-
-
-
-
-
-
-
PASS
-
-
-
-
-
- - diff --git a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002-ref.html b/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002-ref.html deleted file mode 100644 index 417549e9195b..000000000000 --- a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002-ref.html +++ /dev/null @@ -1,59 +0,0 @@ - - - -CSS Content Visibility: auto + overflow clip + scrollIntoView - - - - - - - - -
-
-
-
-
-
PASS
-
-
-
-
-
-
- - - diff --git a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002.html b/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002.html deleted file mode 100644 index 5358fc6db8ae..000000000000 --- a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-002.html +++ /dev/null @@ -1,63 +0,0 @@ - - - -CSS Content Visibility: auto + overflow clip + scrollIntoView - - - - - - - - - -
-
-
-
-
-
PASS
-
-
-
-
-
-
- - diff --git a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html b/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html deleted file mode 100644 index 685dba0b7b43..000000000000 --- a/testing/web-platform/tests/css/css-contain/content-visibility/content-visibility-vs-scrollIntoView-003.html +++ /dev/null @@ -1,81 +0,0 @@ - - - -CSS Content Visibility: auto + overflow clip + scrollIntoView, ContentVisibilityAutoStateChange fires twice - - - - - - - - - -
-
-
-
-
-
PASS
-
-
-
-
-
-
- -