[go: up one dir, main page]

[scroll-snap] Fix bug where snap area was not being removed

Previously we would first check that as area is not a viewport defining
element (VDE) before potentially removing it from its snap container.


The problem is that VDE can change over the lifetime of a document for
example it can initially be HTML and later BODY. This means that it is
possible for the existing logic to consider BODY as a snap area first
and add it to a container but don't consider it a valid snap area at
a later point and thus miss removing it from the map.

The fix simply removes the VDE check for snap areas. We don't really
need to limit VDE being a snap area. We still keep limitation for VDE
becoming a snap container but there removal logic operates regardless so
it is safe.


(cherry picked from commit 86c95f06d09497cb3ac6810f0900ad58b2ab324f)

Bug: 972434
Change-Id: I6d87f8df53f59c58b8302ed7b536b3f6783d98f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1657754
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#669128}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1664477
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#408}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc b/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
index 89d6f8e..984bd807 100644
--- a/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
+++ b/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
@@ -88,12 +88,6 @@
 
 void SnapCoordinator::SnapAreaDidChange(LayoutBox& snap_area,
                                         cc::ScrollSnapAlign scroll_snap_align) {
-  // The viewport defining element cannot be a snap area.
-  if (snap_area.GetNode() ==
-          snap_area.GetDocument().ViewportDefiningElement() ||
-      snap_area.IsLayoutView())
-    return;
-
   LayoutBox* old_container = snap_area.SnapContainer();
   if (scroll_snap_align.alignment_inline == cc::SnapAlignment::kNone &&
       scroll_snap_align.alignment_block == cc::SnapAlignment::kNone) {