Commit b119c2f7 authored by Ella Ge's avatar Ella Ge Committed by Commit Bot

Fix spin button capture and change events.

Spin button was setting capturing_ bit on mouse hover, and clear it on
either mouse up or mouse move but not hover it. It's different from all
other capture behavior, and it causes a bug that capturing_ bit will
not be set to true again when click again without move mouse.

This CL changes the spin button to set capture on mouse down instead of
mouse move (which is align with other capture behavior such as slider
thumb element)

This CL also changes spin button element capturing to using
set/releasePointerCapture api instead of the old method.

Bug: 748073, 919908
Change-Id: I9c501e0b60501707d15f39a5be5efb4e8e12c620
Reviewed-on: https://chromium-review.googlesource.com/c/1432332Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Reviewed-by: default avatarNavid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: default avatarKeishi Hattori <keishi@chromium.org>
Commit-Queue: Ella Ge <eirage@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627009}
parent 4a8c777f
...@@ -110,6 +110,17 @@ void SpinButtonElement::DefaultEventHandler(Event& event) { ...@@ -110,6 +110,17 @@ void SpinButtonElement::DefaultEventHandler(Event& event) {
DoStepAction(up_down_state_ == kUp ? 1 : -1); DoStepAction(up_down_state_ == kUp ? 1 : -1);
} }
} }
// Check |GetLayoutObject| again to make sure element is not removed by
// |DoStepAction|
if (GetLayoutObject() && !capturing_) {
if (LocalFrame* frame = GetDocument().GetFrame()) {
frame->GetEventHandler().SetPointerCapture(
PointerEventFactory::kMouseId, this);
capturing_ = true;
if (Page* page = GetDocument().GetPage())
page->GetChromeClient().RegisterPopupOpeningObserver(this);
}
}
event.SetDefaultHandled(); event.SetDefaultHandled();
} }
} else if (mouse_event.type() == event_type_names::kMouseup && } else if (mouse_event.type() == event_type_names::kMouseup &&
...@@ -118,14 +129,6 @@ void SpinButtonElement::DefaultEventHandler(Event& event) { ...@@ -118,14 +129,6 @@ void SpinButtonElement::DefaultEventHandler(Event& event) {
ReleaseCapture(); ReleaseCapture();
} else if (event.type() == event_type_names::kMousemove) { } else if (event.type() == event_type_names::kMousemove) {
if (box->PixelSnappedBorderBoxRect().Contains(local)) { if (box->PixelSnappedBorderBoxRect().Contains(local)) {
if (!capturing_) {
if (LocalFrame* frame = GetDocument().GetFrame()) {
frame->GetEventHandler().SetCapturingMouseEventsElement(this);
capturing_ = true;
if (Page* page = GetDocument().GetPage())
page->GetChromeClient().RegisterPopupOpeningObserver(this);
}
}
UpDownState old_up_down_state = up_down_state_; UpDownState old_up_down_state = up_down_state_;
up_down_state_ = (local.Y() < box->Size().Height() / 2) ? kUp : kDown; up_down_state_ = (local.Y() < box->Size().Height() / 2) ? kUp : kDown;
if (up_down_state_ != old_up_down_state) if (up_down_state_ != old_up_down_state)
...@@ -191,7 +194,8 @@ void SpinButtonElement::ReleaseCapture(EventDispatch event_dispatch) { ...@@ -191,7 +194,8 @@ void SpinButtonElement::ReleaseCapture(EventDispatch event_dispatch) {
if (!capturing_) if (!capturing_)
return; return;
if (LocalFrame* frame = GetDocument().GetFrame()) { if (LocalFrame* frame = GetDocument().GetFrame()) {
frame->GetEventHandler().SetCapturingMouseEventsElement(nullptr); frame->GetEventHandler().ReleasePointerCapture(
PointerEventFactory::kMouseId, this);
capturing_ = false; capturing_ = false;
if (Page* page = GetDocument().GetPage()) if (Page* page = GetDocument().GetPage())
page->GetChromeClient().UnregisterPopupOpeningObserver(this); page->GetChromeClient().UnregisterPopupOpeningObserver(this);
......
<!DOCTYPE>
<html>
<body>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../../resources/testdriver.js"></script>
<script src="../../../resources/testdriver-vendor.js"></script>
<script src="../../../external/wpt/resources/testdriver-actions.js"></script>
<input type="number" id="num1" onchange="handleChangeEvent(this);" oninput="handleInputEvent(this);">
<div id="div1" style="width:10px;height:10px; margin:50px; background-color: red"></div>
</body>
<script>
var changeEventCounter = 0;
var inputEventCounter = 0;
var mouseMoveCount = 0;
num1.value = 0;
x = num1.offsetLeft + num1.offsetWidth - 10;
y = num1.offsetTop + num1.offsetHeight / 4
function handleChangeEvent(element) {
++changeEventCounter;
}
function handleInputEvent(element) {
++inputEventCounter;
}
div1.addEventListener("pointermove", function(e) {
++mouseMoveCount;
})
promise_test (async() => {
num1.focus();
await new test_driver.Actions()
.pointerMove(x, y)
.pointerDown()
.pointerUp()
.pointerDown()
.pointerUp()
.pointerDown()
.pointerUp()
.send();
assert_equals(inputEventCounter, 3, "input event");
assert_equals(changeEventCounter, 3, "change event");
assert_equals(num1.value, '3', "number field value");
}, "Test that send 3 click without move in between dispatch input and change events correctly");
promise_test (async() => {
await new test_driver.Actions()
.pointerMove(0, 0, {origin: div1})
.pointerDown()
.pointerUp()
.send();
assert_equals(inputEventCounter, 3, "input event");
assert_equals(changeEventCounter, 3, "change event");
assert_equals(num1.value, '3', "number field value");
assert_equals(mouseMoveCount, 1)
}, "pointer move and click elsewhere, and it's not captured by spin button");
promise_test (async() => {
num1.focus();
await new test_driver.Actions()
.pointerMove(x, y)
.pointerDown()
.pointerMove(x - 10, y - 10)
.send();
assert_equals(inputEventCounter, 4, "input event");
assert_equals(changeEventCounter, 4, "change event");
assert_equals(num1.value, '4', "number field value");
}, "Mouse down at spin button and move out dispatch input and change events correctly");
promise_test (async() => {
await new test_driver.Actions()
.pointerMove(0, 0, {origin: div1})
.pointerDown()
.pointerUp()
.send();
assert_equals(inputEventCounter, 4, "input event");
assert_equals(changeEventCounter, 4, "change event");
assert_equals(num1.value, '4', "number field value");
assert_equals(mouseMoveCount, 2)
}, "click elsewhere is not captured by spin button");
</script>
</body>
</html>
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