Commit 8042bc80 authored by Mason Freed's avatar Mason Freed Committed by Commit Bot

Revert "Keep scroll by keyboard after child focused node is removed"

This reverts commit 09a1f53e.

Sheriff: I suspect this is causing the LEAK failures here:
https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20Trusty%20Leak/34655
https://test-results.appspot.com/data/layout_results/WebKit_Linux_Trusty_Leak/34655/webkit_layout_tests/layout-test-results/results.html


Original change's description:
> Keep scroll by keyboard after child focused node is removed
> 
> So far, scroll stops if focused child node is removed
> while scrolling parent node.
> This is because the event handler passes the |MousePressNode|.
> If focused child node(|MousePressNode|) is removed,
> it will be null and scrollable area changed to document.
> 
> |LogicalScroll| finds scrollable area from |MousePressNode|.
> If we save this scrollable area node,
> we can get one more chance to find proper node to scroll.
> 
> 
> Bug: 493078
> Change-Id: I6e81ca0a0d15e0def66c8457840ca9e1343e8dba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1611620
> Commit-Queue: Lan Wei <lanwei@chromium.org>
> Reviewed-by: Lan Wei <lanwei@chromium.org>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#663040}

TBR=bokan@chromium.org,lanwei@chromium.org,bluewhale.marc@gmail.com

Change-Id: I4b8ea440dc16022077746d7924506e7f03ac1673
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 493078
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628880Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663102}
parent c32b0642
...@@ -94,7 +94,6 @@ void ScrollManager::Trace(blink::Visitor* visitor) { ...@@ -94,7 +94,6 @@ void ScrollManager::Trace(blink::Visitor* visitor) {
visitor->Trace(previous_gesture_scrolled_node_); visitor->Trace(previous_gesture_scrolled_node_);
visitor->Trace(scrollbar_handling_scroll_gesture_); visitor->Trace(scrollbar_handling_scroll_gesture_);
visitor->Trace(resize_scrollable_area_); visitor->Trace(resize_scrollable_area_);
visitor->Trace(last_logical_scrolled_node_);
} }
void ScrollManager::ClearGestureScrollState() { void ScrollManager::ClearGestureScrollState() {
...@@ -237,11 +236,6 @@ bool ScrollManager::LogicalScroll(ScrollDirection direction, ...@@ -237,11 +236,6 @@ bool ScrollManager::LogicalScroll(ScrollDirection direction,
if (!node) if (!node)
node = mouse_press_node; node = mouse_press_node;
if ((!node || !node->GetLayoutObject()) &&
(last_logical_scrolled_node_ &&
last_logical_scrolled_node_->GetLayoutObject()))
node = last_logical_scrolled_node_;
if ((!node || !node->GetLayoutObject()) && frame_->View() && if ((!node || !node->GetLayoutObject()) && frame_->View() &&
frame_->View()->GetLayoutView()) frame_->View()->GetLayoutView())
node = frame_->View()->GetLayoutView()->GetNode(); node = frame_->View()->GetLayoutView()->GetNode();
...@@ -325,10 +319,8 @@ bool ScrollManager::LogicalScroll(ScrollDirection direction, ...@@ -325,10 +319,8 @@ bool ScrollManager::LogicalScroll(ScrollDirection direction,
ScrollResult result = scrollable_area->UserScroll( ScrollResult result = scrollable_area->UserScroll(
granularity, ToScrollDelta(physical_direction, 1)); granularity, ToScrollDelta(physical_direction, 1));
if (result.DidScroll()) { if (result.DidScroll())
last_logical_scrolled_node_ = scroll_chain_node;
return true; return true;
}
} }
return false; return false;
......
...@@ -153,8 +153,6 @@ class CORE_EXPORT ScrollManager ...@@ -153,8 +153,6 @@ class CORE_EXPORT ScrollManager
Member<Node> scroll_gesture_handling_node_; Member<Node> scroll_gesture_handling_node_;
Member<Node> last_logical_scrolled_node_;
bool last_gesture_scroll_over_embedded_content_view_; bool last_gesture_scroll_over_embedded_content_view_;
// The most recent Node to scroll natively during this scroll // The most recent Node to scroll natively during this scroll
......
...@@ -2754,7 +2754,6 @@ crbug.com/893490 [ Mac ] external/wpt/css/css-text/white-space/control-chars-09F ...@@ -2754,7 +2754,6 @@ crbug.com/893490 [ Mac ] external/wpt/css/css-text/white-space/control-chars-09F
crbug.com/893480 external/wpt/infrastructure/testdriver/actions/elementTiming.html [ Timeout ] crbug.com/893480 external/wpt/infrastructure/testdriver/actions/elementTiming.html [ Timeout ]
crbug.com/893480 external/wpt/infrastructure/testdriver/actions/multiDevice.html [ Failure Timeout ] crbug.com/893480 external/wpt/infrastructure/testdriver/actions/multiDevice.html [ Failure Timeout ]
crbug.com/893480 external/wpt/infrastructure/testdriver/actions/actionsWithKeyPressed.html [ Failure Timeout ] crbug.com/893480 external/wpt/infrastructure/testdriver/actions/actionsWithKeyPressed.html [ Failure Timeout ]
crbug.com/493078 external/wpt/dom/nodes/keyboard-scroll-removed-node.html [ Failure Timeout ]
# Hit a DCHECK # Hit a DCHECK
crbug.com/918664 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/sizing/block-size-with-min-or-max-content-table-1a.html [ Failure Pass ] crbug.com/918664 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/sizing/block-size-with-min-or-max-content-table-1a.html [ Failure Pass ]
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>Keyboard scroll removed node</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<style>
ul {
width: 30vw;
height: 40vh;
overflow-y: scroll;
}
li {
width: 95%;
height: 10vh;
border: 1px solid black;
}
#target {
background-color: grey;
}
</style>
</head>
<body>
<ul id="list">
<li>ITEM 1</li>
<li>ITEM 2</li>
<li id="target">TARGET ITEM 3</li>
<li>ITEM 4</li>
<li>ITEM 5</li>
<li>ITEM 6</li>
<li>ITEM 7</li>
</ul>
</body>
<script>
async_test(t => {
let listElement = document.getElementById("list");
let targetElement = document.getElementById("target");
let firstScrollTop, secondScrollTop;
let ArrowDownKey = "\uE015";
let firstActions = new test_driver.Actions()
.pointerMove(10, 10, { origin: targetElement })
.pointerDown()
.pointerUp()
.keyDown(ArrowDownKey)
.keyUp(ArrowDownKey)
.send()
.then(() => {
firstScrollTop = listElement.scrollTop;
targetElement.remove();
let secondAction = new test_driver.Actions()
.keyDown(ArrowDownKey)
.keyUp(ArrowDownKey)
.send()
.then(() => {
secondScrollTop = listElement.scrollTop;
assert_greater_than(secondScrollTop, firstScrollTop);
t.done();
})
.catch(t.unreached_func("Actions sequence failed"));
})
.catch(t.unreached_func("Actions sequence failed"));
}, "Keyboard scrolls, after clicked element is removed, continue to affect previous scroller");
</script>
</html>
\ No newline at end of file
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