Commit 5d46b77a authored by Sandra Sun's avatar Sandra Sun Committed by Commit Bot

Snap at ScrollIntoView.

According to the spec,
https://github.com/w3c/csswg-drafts/issues/2593#issuecomment-386154394
scrollIntoView should
1) Always snap to the target element's snap alignment, and
2) All the affected scrollers should also land on a snap position if
one exists.

This patch does part 2). Before a scrollable_area is added to
the smoothScrollSequencer with its new scroll_offset, we will check if
it has a valid snap offset around, and update the final offset
accordingly.

We'll implement part 1) in a separate patch.

Bug: 842317
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
Reviewed-on: https://chromium-review.googlesource.com/c/1188746Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607019}
parent f5e23d9d
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-scroll-snap-1" />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
div {
position: absolute;
}
:root {
overflow: scroll;
scroll-snap-type: both mandatory;
}
.scroller {
overflow: scroll;
scroll-snap-type: both mandatory;
padding: 0px;
}
#outer {
left: 1000px;
top: 1000px;
width: 600px;
height: 600px;
}
#out-snap-1 {
scroll-snap-align: start;
left: 1200px;
top: 1200px;
width: 10px;
height: 10px;
}
#out-snap-2 {
scroll-snap-align: start;
left: 1100px;
top: 1100px;
width: 10px;
height: 10px;
}
#inner {
left: 1000px;
top: 1000px;
width: 400px;
height: 400px;
}
.space {
left: 0px;
top: 0px;
width: 3000px;
height: 3000px;
}
#target {
scroll-snap-align: end;
left: 800px;
top: 800px;
width: 200px;
height: 200px;
}
</style>
<div class="space"></div>
<div id="out-snap-1"></div>
<div id="out-snap-2"></div>
<div class="scroller" id="outer">
<div class="space"></div>
<div class="scroller" id="inner">
<div class="space"></div>
<div id="target"></div>
</div>
</div>
<script>
var outer = document.getElementById("outer");
var inner = document.getElementById("inner");
var target = document.getElementById("target");
test(() => {
assert_equals(window.scrollX, 0);
assert_equals(window.scrollY, 0);
assert_equals(outer.scrollLeft, 0);
assert_equals(outer.scrollTop, 0);
assert_equals(inner.scrollLeft, 0);
assert_equals(inner.scrollTop, 0);
target.scrollIntoView({inline: "start", block: "start"});
// Although the scrollIntoView specified "start" as the alignment, the target
// has "end" as its snap-alignment. So the inner scroller finishes with "end"
// alignment
assert_equals(inner.scrollLeft, 1000 - inner.clientWidth, "ScrollIntoView lands on the target's snap position regardless of the specified alignment.");
assert_equals(inner.scrollTop, 1000 - inner.clientHeight, "ScrollIntoView lands on the target's snap position regardless of the specified alignment.");
// Since there is no snap points defined in the outer scroller, the outer
// scroller finishes with "start" alignment, as specified in scrollIntoView.
// Note that the "start" alignment aligns the target's top-left corner
//(inner.left + inner.clientWidth - target.width, inner.top + inner.clientHeight - target.height)
// with the outer scroller's top-left corner.
assert_equals(outer.scrollLeft, 800 + inner.clientWidth, "ScrollIntoView ends with the specified alignment if no snap position is specified.");
assert_equals(outer.scrollTop, 800 + inner.clientHeight, "ScrollIntoView ends with the specified alignment if no snap position is specified.");
// Although the scrollIntoView specified "start" as the alignment, the window
// has two other elements with snap points. So the window finishes with the
// closest snap point.
assert_equals(window.scrollX, 1100, "ScrollIntoView lands on the snap position closest to the specified alignment.");
assert_equals(window.scrollY, 1100, "ScrollIntoView lands on the snap position closest to the specified alignment.");
}, "All the scrollers affected by scrollIntoView should land on a snap position if one exists. Otherwise, land according to the specified alignment");
</script>
\ No newline at end of file
......@@ -8,6 +8,7 @@
#include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/layout/layout_box.h"
#include "third_party/blink/renderer/core/layout/scroll_anchor.h"
#include "third_party/blink/renderer/core/page/scrolling/snap_coordinator.h"
#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
#include "third_party/blink/renderer/core/scroll/scroll_animator_base.h"
#include "third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h"
......@@ -293,6 +294,19 @@ LayoutRect RootFrameViewport::ScrollIntoView(
if (params.GetScrollType() == kUserScroll)
new_scroll_offset = ClampToUserScrollableOffset(new_scroll_offset);
FloatPoint end_point = ScrollOffsetToPosition(new_scroll_offset);
std::unique_ptr<SnapSelectionStrategy> strategy =
SnapSelectionStrategy::CreateForEndPosition(gfx::ScrollOffset(end_point),
true, true);
if (GetLayoutBox()) {
end_point = GetLayoutBox()
->GetDocument()
.GetSnapCoordinator()
->GetSnapPosition(*GetLayoutBox(), *strategy)
.value_or(end_point);
new_scroll_offset = ScrollPositionToOffset(end_point);
}
if (new_scroll_offset != GetScrollOffset()) {
if (params.is_for_scroll_sequence) {
DCHECK(params.GetScrollType() == kProgrammaticScroll ||
......
......@@ -2038,6 +2038,18 @@ LayoutRect PaintLayerScrollableArea::ScrollIntoView(
if (!UserInputScrollable(kVerticalScrollbar))
new_scroll_offset.SetHeight(old_scroll_offset.Height());
}
FloatPoint end_point = ScrollOffsetToPosition(new_scroll_offset);
std::unique_ptr<SnapSelectionStrategy> strategy =
SnapSelectionStrategy::CreateForEndPosition(gfx::ScrollOffset(end_point),
true, true);
end_point = GetLayoutBox()
->GetDocument()
.GetSnapCoordinator()
->GetSnapPosition(*GetLayoutBox(), *strategy)
.value_or(end_point);
new_scroll_offset = ScrollPositionToOffset(end_point);
if (params.is_for_scroll_sequence) {
DCHECK(params.GetScrollType() == kProgrammaticScroll ||
params.GetScrollType() == kUserScroll);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment