Skip to content

Commit

Permalink
Revert "[soft navigations] Enable keyboard shortcuts as soft navigati…
Browse files Browse the repository at this point in the history
…on triggers"

This reverts commit 6efe71286a014d3d3872bc990e3ea2d08dd46dba.

Reason for revert: One check added in this CL causes crash. see
crbug.com/1480047

Original change's description:
> [soft navigations] Enable keyboard shortcuts as soft navigation triggers
>
> Following the discussion on issue #3 [1], this CL adds support to soft
> navigations triggered by keyboard shortcuts, by adding unfocused keydown
> events to the events that can trigger the soft navigation heuristic.
>
> [1] WICG/soft-navigations#3
>
> Bug: 1478772
> Change-Id: Ib423a3cfc09eaf4dd9a2221b3494ab1016fa8668
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4839506
> Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
> Reviewed-by: Ian Clelland <iclelland@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1193004}

Bug: 1478772
Change-Id: I3a518c165e6b19239a6bf7900e94c1ef9c3e5a5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4859802
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Hao Liu <haoliuk@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1196100}
  • Loading branch information
haoliuk authored and chromium-wpt-export-bot committed Sep 13, 2023
1 parent a4934f7 commit 4054394
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 54 deletions.
Expand Up @@ -31,7 +31,7 @@

const first_click_paint_promise = waitOnPaintEntriesPromise();

interact(link);
click(link);
await new Promise(resolve => {
(new PerformanceObserver(resolve)).observe({
type: 'soft-navigation'
Expand All @@ -54,7 +54,7 @@

const second_click_paint_promise = waitOnPaintEntriesPromise();
const preClickLcp2 = await getLcpEntries();
interact(link);
click(link);
await new Promise(resolve => {
(new PerformanceObserver(() => resolve())).observe({
type: 'soft-navigation'
Expand Down
30 changes: 0 additions & 30 deletions soft-navigation-heuristics/keydown.tentative.html

This file was deleted.

2 changes: 1 addition & 1 deletion soft-navigation-heuristics/navigate-child.html
Expand Up @@ -20,7 +20,7 @@
await new Promise(r => t.step_timeout(r, 10));
}
const link = document.getElementById("link");
interact(link);
click(link);
while (!child.location.href.includes("2")) {
await new Promise(r => t.step_timeout(r, 10));
}
Expand Down
35 changes: 14 additions & 21 deletions soft-navigation-heuristics/resources/soft-navigation-helper.js
@@ -1,5 +1,5 @@
var counter = 0;
var interacted;
var clicked;
var timestamps = []
const MAX_CLICKS = 50;
// Entries for one hard navigation + 50 soft navigations.
Expand All @@ -20,7 +20,6 @@ const testSoftNavigation =
const testName = options.testName;
const pushUrl = readValue(options.pushUrl, true);
const eventType = readValue(options.eventType, "click");
const interactionType = readValue(options.interactionType, 'click');
const expectLCP = options.validate != 'no-lcp';
promise_test(async t => {
await waitInitialLCP();
Expand All @@ -30,8 +29,8 @@ const testSoftNavigation =
const firstClick = (i === 0);
let paint_entries_promise =
waitOnPaintEntriesPromise(expectLCP && firstClick);
interacted = false;
interact(link, interactionType);
clicked = false;
click(link);

await new Promise(resolve => {
(new PerformanceObserver(() => resolve())).observe({
Expand Down Expand Up @@ -62,7 +61,7 @@ const testNavigationApi = (testName, navigateEventHandler, link) => {
await waitInitialLCP();
const preClickLcp = await getLcpEntries();
let paint_entries_promise = waitOnPaintEntriesPromise();
interact(link);
click(link);
await new Promise(resolve => {
(new PerformanceObserver(() => resolve())).observe({
type: 'soft-navigation'
Expand All @@ -81,7 +80,7 @@ const testSoftNavigationNotDetected = options => {
promise_test(async t => {
const preClickLcp = await getLcpEntries();
options.eventTarget.addEventListener(options.eventName, options.eventHandler);
interact(options.link);
click(options.link);
await new Promise((resolve, reject) => {
(new PerformanceObserver(() =>
reject("Soft navigation should not be triggered"))).observe({
Expand Down Expand Up @@ -129,21 +128,15 @@ const runEntryValidations =
}
};

const interact =
(link, interactionType = 'click') => {
if (test_driver) {
if (interactionType == 'click') {
test_driver.click(link);
} else {
test_driver.send_keys(link, 'j');
}
timestamps[counter] = {"syncPostInteraction": performance.now()};
}
}
const click = link => {
if (test_driver) {
test_driver.click(link);
timestamps[counter] = {"syncPostClick": performance.now()};
}
}

const setEvent = (t, button, pushState, addContent, pushUrl, eventType) => {
const eventObject =
(eventType == 'click' || eventType == 'keydown') ? button : window;
const eventObject = (eventType == "click") ? button : window;
eventObject.addEventListener(eventType, async e => {
timestamps[counter]["eventStart"] = performance.now();
// Jump through a task, to ensure task tracking is working properly.
Expand All @@ -165,7 +158,7 @@ const setEvent = (t, button, pushState, addContent, pushUrl, eventType) => {
await addContent(url);
++counter;

interacted = true;
clicked = true;
});
};

Expand All @@ -185,7 +178,7 @@ const validateSoftNavigationEntry = async (clicks, extraValidations,
assert_true(entry.name.includes(pushUrl ? URL : document.location.href),
"The soft navigation name is properly set");
const entryTimestamp = entry.startTime;
assert_less_than_equal(timestamps[i]["syncPostInteraction"], entryTimestamp);
assert_less_than_equal(timestamps[i]["syncPostClick"], entryTimestamp);
assert_greater_than_equal(
timestamps[i]['eventStart'], entryTimestamp,
'Event start timestamp matches');
Expand Down

0 comments on commit 4054394

Please sign in to comment.