Skip to content

GitLab

T
tangled
Commit 561ebc9c authored by Mario Bianucci's avatar Mario Bianucci Committed by Commit Bot
Browse files
Options

Text should remain selected when scrolling with middle mouse button 

Currently, if you select a text on the page, hold down middle mouse
button and start scrolling in either direction and leave the button, it
deselects the text. This is due to incorrect position in widget
information on an event.

This CL fixes the bug by correctly populating the position in widget
information on an event.

As a side effect of populating the correct position, clicking in an
OOPIF to end autoscrolling would result in the cursor not changing back
to a pointer until the mouse was moved. This is because when auto-
scrolling ends, a cursor update is scheduled, but the cursor update
timer is tied to user interaction, and if user interaction doesn't
occur after the click, the timer doesn't fire. This used to work because
the cached value previously populated in the widget resulted in hit
testing not reporting a remote frame, so the cursor was updated in
EventHandler::HandleMouseMoveOrLeaveEvent. With this change, hit testing
reports a remote frame, so it no longer updates in the same place.
Therefore, in order to ensure that the cursor is correctly updated
immediately after autoscrolling, CursorUpdate is called rather than
just scheduling an update.

Regressing change: crrev.com/c/1773540

Bug: 1054854
Change-Id: I6b5353529ab0b73c85c1dd0e0237d3e06c848a9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116134Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Reviewed-by: default avatarDaniel Libby <dlibby@microsoft.com>
Commit-Queue: Mario Bianucci <mabian@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#756405}
parent 565178de
Hide whitespace changes
Inline Side-by-side
Showing with 69 additions and 3 deletions
content/browser/renderer_host/render_widget_targeter.cc
View file @ 561ebc9c
......@@ -226,6 +226,13 @@ void RenderWidgetTargeter::ResolveTargetingRequest(TargetingRequest request) {
? middle_click_result_
: delegate_->FindTargetSynchronously(request_target,
*request.GetEvent());
// |result.target_location| is utilized to update the position in widget for
// an event. If we are in autoscroll mode, we used cached data. So we need
// to update the target location of the |result|.
if (is_autoscroll_in_progress_) {
result.target_location = request_target_location;
}
if (!is_autoscroll_in_progress_ &&
IsMouseMiddleClick(*request.GetEvent())) {
if (!result.should_query_view)
......
third_party/blink/renderer/core/input/event_handler.h
View file @ 561ebc9c
......@@ -270,6 +270,8 @@ class CORE_EXPORT EventHandler final : public GarbageCollected<EventHandler> {
bool LongTapShouldInvokeContextMenu();
void UpdateCursor();
private:
WebInputEventResult HandleMouseMoveOrLeaveEvent(
const WebMouseEvent&,
......@@ -303,8 +305,6 @@ class CORE_EXPORT EventHandler final : public GarbageCollected<EventHandler> {
void CursorUpdateTimerFired(TimerBase*);
void ActiveIntervalTimerFired(TimerBase*);
void UpdateCursor();
ScrollableArea* AssociatedScrollableArea(const PaintLayer*) const;
Element* EffectiveMouseEventTargetElement(Element*);
......
third_party/blink/renderer/core/page/autoscroll_controller.cc
View file @ 561ebc9c
......@@ -323,7 +323,7 @@ void AutoscrollController::StopMiddleClickAutoscroll(LocalFrame* frame) {
page_->GetChromeClient().AutoscrollEnd(frame);
autoscroll_type_ = kNoAutoscroll;
page_->GetChromeClient().SetCursorOverridden(false);
frame->LocalFrameRoot().GetEventHandler().ScheduleCursorUpdate();
frame->LocalFrameRoot().GetEventHandler().UpdateCursor();
autoscroll_layout_object_ = nullptr;
}
......
third_party/blink/web_tests/fast/events/autoscroll-keeps-selection.html 0 → 100644
View file @ 561ebc9c
<!DOCTYPE HTML>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../../resources/gesture-util.js"></script>
<script>
function start() {
promise_test(async () => {
assert_equals(document.getSelection().type,"None");
var scrollable = document.getElementById('scrollable');
// Highlight some of the text
document.getSelection().setBaseAndExtent(scrollable,0,scrollable,2);
assert_equals(document.getSelection().toString(),
"This is highlightable and scrollable text");
const middle_click_autoscroll_radius = 15; // from middleClickAutoscroll.js
const middle_button = 1;
const waitTimeBeforeMoveInMilliseconds = 100;
// X can be the same for start and end because only vertical autoscrolling
// is being tested
var start_x = end_x = scrollable.offsetLeft + 5;
var start_y = scrollable.offsetTop + 5;
var end_y = scrollable.offsetTop + middle_click_autoscroll_radius + 6;
await mouseDragAndDrop(start_x, start_y, end_x, end_y, middle_button,
waitTimeBeforeMoveInMilliseconds);
// Confirm that scrolling did happen and the same text is selected.
assert_true(scrollable.scrollTop > 0);
assert_equals(document.getSelection().toString(),
"This is highlightable and scrollable text");
}, 'Holding middle button to autoscroll does not clear selected text');
}
</script>
<style>
#scrollable {
height: 100px;
width: 400px;
overflow: scroll;
}
</style>
<body onload="start()">
<div id="scrollable">
<span>This is highlightable and scrollable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
<span>Different scrollable and highlightable text</span>
</div>
</body>
\ 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