Skip to content

Commit

Permalink
Revert "Add a WPT for pointerevent user activation trigger."
Browse files Browse the repository at this point in the history
This reverts commit 34b3146.

Reason for revert: Suspected cause of crbug.com/1269035, will
verify before submitting.

Original change's description:
> Add a WPT for pointerevent user activation trigger.
>
> Related to HTML issue: whatwg/html#3849
>
> Also fix double-activation in Chrome's gesture tap.
>
> Bug: 826293, 1265587
> Change-Id: I3bc3bcbcfeda242512fb777a1c8881c475fed5fc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3254213
> Reviewed-by: Robert Flack <flackr@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#940118}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 826293, 1265587
Change-Id: I4bc041f2254c382c00ce3596d61742ba84c79660
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3277061
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Johann Koenig <johannkoenig@google.com>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Johann Koenig <johannkoenig@google.com>
Owners-Override: Johann Koenig <johannkoenig@google.com>
Cr-Commit-Position: refs/heads/main@{#940996}
  • Loading branch information
Brian Sheedy authored and Chromium LUCI CQ committed Nov 12, 2021
1 parent e6e0d04 commit 9872337
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 122 deletions.
12 changes: 1 addition & 11 deletions chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -758,17 +758,7 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, TapGestureWithCtrlKey) {
#else
unsigned modifiers = blink::WebInputEvent::kControlKey;
#endif

// The tap simulators in browser_test_utils doesn't fully reflect real tap
// interactions because unlike real taps, they don't sent touch/pointer events
// before a tap hence miss the user activation from touchend/pointerup event.
// To simulate the missing user activation, we are sending a no-op keypress
// (SPACE) right before the simulated tap.
SimulateKeyPress(tab, ui::DomKey::FromCharacter(' '), ui::DomCode::SPACE,
ui::VKEY_SPACE,
/*control=*/false, /*shift=*/false, /*alt=*/false,
/*command=*/false);
SimulateTapWithModifiersAt(tab, modifiers, gfx::Point(350, 250));
content::SimulateTapWithModifiersAt(tab, modifiers, gfx::Point(350, 250));

tab_add.Wait();

Expand Down
7 changes: 3 additions & 4 deletions third_party/blink/renderer/core/input/gesture_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ WebInputEventResult GestureManager::HandleGestureTap(
FlooredIntPoint(gesture_event.PositionInRootFrame());
Node* tapped_node = current_hit_test.InnerNode();
Element* tapped_element = current_hit_test.InnerElement();
LocalFrame::NotifyUserActivation(
tapped_node ? tapped_node->GetDocument().GetFrame() : nullptr,
mojom::blink::UserActivationNotificationType::kInteraction);

mouse_event_manager_->SetClickElement(tapped_element);

Expand Down Expand Up @@ -400,10 +403,6 @@ WebInputEventResult GestureManager::HandleGestureLongPress(
return WebInputEventResult::kNotHandled;
}

// For touch pointer interactions, pointerup is an activation triggering
// event. A long-press gesture doesn't fire a pointerup event (fires a
// pointercancel instead), but for compat reasons we need user activation
// here.
LocalFrame::NotifyUserActivation(
inner_node ? inner_node->GetDocument().GetFrame() : nullptr,
mojom::blink::UserActivationNotificationType::kInteraction);
Expand Down
39 changes: 5 additions & 34 deletions third_party/blink/renderer/core/input/ime_on_focus_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,40 +39,23 @@ class ImeOnFocusTest : public testing::Test {
const AtomicString& focus_element = g_null_atom,
String frame = "");

private:
std::unique_ptr<WebPointerEvent> CreateTouchPointerEvent(
WebInputEvent::Type type,
gfx::PointF position,
float size);
String base_url_;
frame_test_helpers::WebViewHelper web_view_helper_;
Persistent<Document> document_;
};

void ImeOnFocusTest::SendGestureTap(WebViewImpl* web_view,
gfx::Point client_point) {
gfx::PointF position(client_point);
float size = 10;

// GestureTap comes after a pointerdown/up.
web_view->MainFrameWidget()->HandleInputEvent(WebCoalescedInputEvent(
CreateTouchPointerEvent(WebInputEvent::Type::kPointerDown, position,
size),
{}, {}, ui::LatencyInfo()));

web_view->MainFrameWidget()->HandleInputEvent(WebCoalescedInputEvent(
CreateTouchPointerEvent(WebInputEvent::Type::kPointerUp, position, size),
{}, {}, ui::LatencyInfo()));

WebGestureEvent web_gesture_event(WebInputEvent::Type::kGestureTap,
WebInputEvent::kNoModifiers,
WebInputEvent::GetStaticTimeStampForTests(),
WebGestureDevice::kTouchscreen);
web_gesture_event.SetPositionInWidget(position);
web_gesture_event.SetPositionInScreen(position);
// GestureTap is only ever from touch screens.
web_gesture_event.SetPositionInWidget(gfx::PointF(client_point));
web_gesture_event.SetPositionInScreen(gfx::PointF(client_point));
web_gesture_event.data.tap.tap_count = 1;
web_gesture_event.data.tap.width = size;
web_gesture_event.data.tap.height = size;
web_gesture_event.data.tap.width = 10;
web_gesture_event.data.tap.height = 10;

web_view->MainFrameViewWidget()->HandleInputEvent(
WebCoalescedInputEvent(web_gesture_event, ui::LatencyInfo()));
Expand Down Expand Up @@ -130,18 +113,6 @@ void ImeOnFocusTest::RunImeOnFocusTest(
web_view_helper_.Reset();
}

std::unique_ptr<WebPointerEvent> ImeOnFocusTest::CreateTouchPointerEvent(
WebInputEvent::Type type,
gfx::PointF position,
float size) {
return std::make_unique<WebPointerEvent>(
type,
WebPointerProperties(1, WebPointerProperties::PointerType::kTouch,
WebPointerProperties::Button::kLeft, position,
position, 0, 0),
size, size);
}

TEST_F(ImeOnFocusTest, OnLoad) {
RunImeOnFocusTest("ime-on-focus-on-load.html", 0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,11 @@ WebInputEventResult PointerEventManager::HandlePointerEvent(
// Any finger lifting is a user gesture only when it wasn't associated with a
// scroll.
// https://docs.google.com/document/d/1oF1T3O7_E4t1PYHV6gyCwHxOi3ystm0eSL5xZu7nvOg/edit#
// For the rare case of multi-finger scenarios spanning documents, it seems
// extremely unlikely to matter which document the gesture is associated with
// so just pick the pointer event that comes.
// Re-use the same UserGesture for touchend and pointerup (but not for the
// mouse events generated by GestureTap).
// For the rare case of multi-finger scenarios spanning documents, it
// seems extremely unlikely to matter which document the gesture is
// associated with so just pick the pointer event that comes.
if (event.GetType() == WebInputEvent::Type::kPointerUp &&
!non_hovering_pointers_canceled_ && pointer_event_target.target_frame) {
LocalFrame::NotifyUserActivation(
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -3283,7 +3283,6 @@ crbug.com/626703 external/wpt/css/css-grid/grid-model/grid-areas-overflowing-gri
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-002.html [ Failure ]
crbug.com/626703 [ Mac10.14 ] external/wpt/pointerevents/pointerevent_pointercapture_in_frame.html?pen [ Timeout ]
crbug.com/626703 [ Mac11 ] external/wpt/pointerevents/pointerevent_pointercapture_in_frame.html?pen [ Timeout ]
crbug.com/1265587 external/wpt/html/user-activation/activation-trigger-pointerevent.html?pen [ Failure ]
crbug.com/626703 external/wpt/html/cross-origin-opener-policy/iframe-popup-same-origin-to-unsafe-none.https.html [ Skip Timeout ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-006.html [ Failure ]
crbug.com/626703 external/wpt/css/css-sizing/fit-content-length-percentage-007.html [ Failure ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ <h1>Test for right-click activation trigger</h1>
<script>
promise_test(async () => {
var actions = new test_driver.Actions();
actions.pointerMove(0, 0, {origin: document.body})
actions_promise = actions
.pointerMove(0, 0, {origin: document.body})
.pointerDown({button: actions.ButtonType.RIGHT})
.pointerUp({button: actions.ButtonType.RIGHT})
.send();
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,8 @@
var bounds = document.querySelector('#target').getBoundingClientRect();
var x = bounds.left + bounds.width / 2;
var y = bounds.top + bounds.height / 2;

// TODO(beccahughes): for some reasons, using pointerActanionSequence fails.
eventSender.clearTouchPoints();
eventSender.addTouchPoint(x, y);
eventSender.touchStart();

eventSender.releaseTouchPoint(0);
eventSender.touchEnd();
eventSender.gestureTap(x, y);

document.location.href = 'resources/test-autoplay.html';
</script>

0 comments on commit 9872337

Please sign in to comment.