Skip to content

Commit

Permalink
App history: basic focusReset support
Browse files Browse the repository at this point in the history
Based on the draft at WICG/navigation-api#201.

This includes handling multiple conflicting focusReset options, with a console warning if necessary.

Limitations, to be addressed in future commits:

* This does not take into account autofocus="" elements.

* This will reset focus even if the user has moved focus in the meantime.

Bug: 1183545
Change-Id: I1eae4fc58af653fa7e463a1346a9bebc9536a59f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3441109
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#970838}
NOKEYCHECK=True
GitOrigin-RevId: d9ba2f92e64c9c06554722bf4c767b3004bbbe3c
  • Loading branch information
domenic authored and copybara-github committed Feb 14, 2022
1 parent eef28e4 commit 0d46f1c
Show file tree
Hide file tree
Showing 12 changed files with 336 additions and 8 deletions.
4 changes: 4 additions & 0 deletions blink/renderer/bindings/generated_in_core.gni
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ generated_dictionary_sources_in_core = [
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_result.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_transition.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_transition.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_transition_while_options.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_transition_while_options.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_update_current_options.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_update_current_options.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_assigned_nodes_options.cc",
Expand Down Expand Up @@ -549,6 +551,8 @@ generated_interface_sources_in_core = [
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_destination.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_entry.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_entry.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_focus_reset.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_focus_reset.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_navigate_event.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_navigate_event.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_app_history_navigation_type.cc",
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/bindings/idl_in_core.gni
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ static_idl_files_in_core = get_path_info(
"//third_party/blink/renderer/core/app_history/app_history_reload_options.idl",
"//third_party/blink/renderer/core/app_history/app_history_result.idl",
"//third_party/blink/renderer/core/app_history/app_history_transition.idl",
"//third_party/blink/renderer/core/app_history/app_history_transition_while_options.idl",
"//third_party/blink/renderer/core/app_history/app_history_update_current_options.idl",
"//third_party/blink/renderer/core/app_history/window_app_history.idl",
"//third_party/blink/renderer/core/clipboard/data_transfer.idl",
Expand Down
18 changes: 14 additions & 4 deletions blink/renderer/core/app_history/app_history.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,26 @@ class NavigateReaction final : public ScriptFunction::Callable {
ScriptPromise promise,
AppHistoryApiNavigation* navigation,
AbortSignal* signal,
bool should_reset_focus,
ReactType react_type) {
promise.Then(MakeGarbageCollected<ScriptFunction>(
script_state, MakeGarbageCollected<NavigateReaction>(
navigation, signal,
navigation, signal, should_reset_focus,
ResolveType::kFulfill, react_type)),
MakeGarbageCollected<ScriptFunction>(
script_state, MakeGarbageCollected<NavigateReaction>(
navigation, signal, ResolveType::kReject,
react_type)));
navigation, signal, should_reset_focus,
ResolveType::kReject, react_type)));
}

NavigateReaction(AppHistoryApiNavigation* navigation,
AbortSignal* signal,
bool should_reset_focus,
ResolveType resolve_type,
ReactType react_type)
: navigation_(navigation),
signal_(signal),
should_reset_focus_(should_reset_focus),
resolve_type_(resolve_type),
react_type_(react_type) {}

Expand All @@ -91,6 +94,11 @@ class NavigateReaction final : public ScriptFunction::Callable {
app_history->RejectPromisesAndFireNavigateErrorEvent(navigation_, value);
}

if (should_reset_focus_) {
// TODO(domenic): use the autofocus delegate, if one exists.
app_history->GetSupplementable()->document()->ClearFocusedElement();
}

if (react_type_ == ReactType::kTransitionWhile && window->GetFrame()) {
window->GetFrame()->Loader().DidFinishNavigation(
resolve_type_ == ResolveType::kFulfill
Expand All @@ -104,6 +112,7 @@ class NavigateReaction final : public ScriptFunction::Callable {
private:
Member<AppHistoryApiNavigation> navigation_;
Member<AbortSignal> signal_;
bool should_reset_focus_;
ResolveType resolve_type_;
ReactType react_type_;
};
Expand Down Expand Up @@ -707,7 +716,8 @@ AppHistory::DispatchResult AppHistory::DispatchNavigateEvent(

NavigateReaction::React(
script_state, ScriptPromise::All(script_state, tweaked_promise_list),
ongoing_navigation_, navigate_event->signal(), react_type);
ongoing_navigation_, navigate_event->signal(),
navigate_event->ShouldResetFocus(), react_type);
} else if (ongoing_navigation_) {
// The spec assumes it's ok to leave a promise permanently unresolved, but
// ScriptPromiseResolver requires either resolution or explicit detach.
Expand Down
41 changes: 38 additions & 3 deletions blink/renderer/core/app_history/app_history_navigate_event.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@

#include "third_party/blink/renderer/core/app_history/app_history_navigate_event.h"

#include "third_party/blink/public/mojom/devtools/console_message.mojom-shared.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_app_history_navigate_event_init.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_app_history_transition_while_options.h"
#include "third_party/blink/renderer/core/app_history/app_history_destination.h"
#include "third_party/blink/renderer/core/dom/abort_signal.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/event_interface_names.h"
#include "third_party/blink/renderer/core/event_type_names.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/html/forms/form_data.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"

namespace blink {

Expand All @@ -35,9 +39,11 @@ AppHistoryNavigateEvent::AppHistoryNavigateEvent(
DCHECK(IsA<LocalDOMWindow>(context));
}

void AppHistoryNavigateEvent::transitionWhile(ScriptState* script_state,
ScriptPromise newNavigationAction,
ExceptionState& exception_state) {
void AppHistoryNavigateEvent::transitionWhile(
ScriptState* script_state,
ScriptPromise newNavigationAction,
AppHistoryTransitionWhileOptions* options,
ExceptionState& exception_state) {
if (!DomWindow()) {
exception_state.ThrowDOMException(
DOMExceptionCode::kInvalidStateError,
Expand Down Expand Up @@ -78,6 +84,35 @@ void AppHistoryNavigateEvent::transitionWhile(ScriptState* script_state,
}

navigation_action_promises_list_.push_back(newNavigationAction);

if (options->hasFocusReset()) {
if (focus_reset_behavior_ &&
focus_reset_behavior_->AsEnum() != options->focusReset().AsEnum()) {
GetExecutionContext()->AddConsoleMessage(
MakeGarbageCollected<ConsoleMessage>(
mojom::ConsoleMessageSource::kJavaScript,
mojom::blink::ConsoleMessageLevel::kWarning,
"The \"" + options->focusReset().AsString() +
"\" value for transitionWhile()'s focusReset option will "
"override the previously-passed value of \"" +
focus_reset_behavior_->AsString() + "\"."));
}
focus_reset_behavior_ = options->focusReset();
}
}

bool AppHistoryNavigateEvent::ShouldResetFocus() const {
// We only do focus reset if transitionWhile() was called, opting us into the
// new default behavior which app history provides.
if (navigation_action_promises_list_.IsEmpty())
return false;

// If we're in "app history mode" per the above, then either leaving focus
// reset behavior as the default, or setting it to "after-transition"
// explicitly, should reset the focus.
return !focus_reset_behavior_ ||
focus_reset_behavior_->AsEnum() ==
V8AppHistoryFocusReset::Enum::kAfterTransition;
}

const AtomicString& AppHistoryNavigateEvent::InterfaceName() const {
Expand Down
6 changes: 6 additions & 0 deletions blink/renderer/core/app_history/app_history_navigate_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_APP_HISTORY_APP_HISTORY_NAVIGATE_EVENT_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_APP_HISTORY_APP_HISTORY_NAVIGATE_EVENT_H_

#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/web/web_frame_load_type.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/core/v8/serialization/serialized_script_value.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_app_history_focus_reset.h"
#include "third_party/blink/renderer/core/app_history/app_history.h"
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
Expand All @@ -21,6 +23,7 @@ namespace blink {
class AbortSignal;
class AppHistoryDestination;
class AppHistoryNavigateEventInit;
class AppHistoryTransitionWhileOptions;
class ExceptionState;
class FormData;
class ScriptPromise;
Expand Down Expand Up @@ -53,11 +56,13 @@ class AppHistoryNavigateEvent final : public Event,

void transitionWhile(ScriptState*,
ScriptPromise newNavigationAction,
AppHistoryTransitionWhileOptions*,
ExceptionState&);

const HeapVector<ScriptPromise>& GetNavigationActionPromisesList() {
return navigation_action_promises_list_;
}
bool ShouldResetFocus() const;

const AtomicString& InterfaceName() const final;
void Trace(Visitor*) const final;
Expand All @@ -71,6 +76,7 @@ class AppHistoryNavigateEvent final : public Event,
Member<AbortSignal> signal_;
Member<FormData> form_data_;
ScriptValue info_;
absl::optional<V8AppHistoryFocusReset> focus_reset_behavior_ = absl::nullopt;

KURL url_;
HeapVector<ScriptPromise> navigation_action_promises_list_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@
readonly attribute FormData? formData;
readonly attribute any info;

[CallWith=ScriptState, RaisesException] void transitionWhile(Promise<any> newNavigationAction);
[CallWith=ScriptState, RaisesException] void transitionWhile(Promise<void> newNavigationAction, optional AppHistoryTransitionWhileOptions options = {});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
dictionary AppHistoryTransitionWhileOptions {
AppHistoryFocusReset focusReset;
};

enum AppHistoryFocusReset {
"after-transition",
"manual"
};
64 changes: 64 additions & 0 deletions blink/web_tests/external/wpt/app-history/focus-reset/basic.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<script type="module">
import { testFocusWasReset, testFocusWasNotReset } from "./resources/helpers.mjs";

test(() => {
let throwAssertionHappened = false;

appHistory.addEventListener("navigate", e => {
assert_throws_js(TypeError, () => {
e.transitionWhile(Promise.resolve(), { focusReset: "invalid" });
});
throwAssertionHappened = true;
}, { once: true });

appHistory.navigate("#1");
assert_true(throwAssertionHappened);
}, "Invalid values for focusReset throw");

testFocusWasNotReset(() => {
// Intentionally left blank.
}, "Does not reset the focus when no navigate handler is present");

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve());
}, { once: true });
}, "Resets the focus when no focusReset option is provided");

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve());
}, { once: true });
}, "Resets the focus when focusReset is explicitly set to undefined");

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(new Promise(r => t.step_timeout(r, 5)));
}, { once: true });
}, "Resets the focus when no focusReset option is provided (nontrivial fulfilled promise)");

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.reject());
}, { once: true });
}, "Resets the focus when no focusReset option is provided (rejected promise)");

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(
Promise.resolve(),
{ focusReset: "after-transition" }
);
}, { once: true });
}, "Resets the focus when focusReset is explicitly set to 'after-transition'");

testFocusWasNotReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "manual" });
});
}, "Does not reset the focus when focusReset is set to 'manual'");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<script type="module">
import { testFocusWasReset, testFocusWasNotReset } from "./resources/helpers.mjs";

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve());
}, { once: true });

appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "after-transition" });
}, { once: true });
}, "(not provided) + after-transition");

testFocusWasNotReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve());
}, { once: true });

appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "manual" });
}, { once: true });
}, "(not provided) + manual");

testFocusWasNotReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "after-transition" });
}, { once: true });

appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "manual" });
}, { once: true });
}, "after-transition + manual");

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "after-transition" });
}, { once: true });

appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve());
}, { once: true });
}, "after-transition + (not provided)");

testFocusWasReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "manual" });
}, { once: true });

appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "after-transition" });
}, { once: true });
}, "manual + after-transition");

testFocusWasNotReset(t => {
appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve(), { focusReset: "manual" });
}, { once: true });

appHistory.addEventListener("navigate", e => {
e.transitionWhile(Promise.resolve());
}, { once: true });
}, "manual + (not provided)");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Usage note: if you use these more than once in a given file, be sure to
// clean up any navigate event listeners, e.g. by using { once: true }, between
// tests.

export function testFocusWasReset(setupFunc, description) {
promise_test(async t => {
setupFunc(t);

const button = document.body.appendChild(document.createElement("button"));
t.add_cleanup(() => { button.remove(); });

assert_equals(document.activeElement, document.body, "Start on body");
button.focus();
assert_equals(document.activeElement, button, "focus() worked");

const { committed, finished } = appHistory.navigate("#" + location.hash.substring(1) + "1");

await committed;
assert_equals(document.activeElement, button, "Focus stays on the button during the transition");

await finished.catch(() => {});
assert_equals(document.activeElement, document.body, "Focus reset after the transition");
}, description);
}

export function testFocusWasNotReset(setupFunc, description) {
promise_test(async t => {
setupFunc(t);

const button = document.body.appendChild(document.createElement("button"));
t.add_cleanup(() => { button.remove(); });

assert_equals(document.activeElement, document.body, "Start on body");
button.focus();
assert_equals(document.activeElement, button, "focus() worked");

const { committed, finished } = appHistory.navigate("#" + location.hash.substring(1) + "1");

await committed;
assert_equals(document.activeElement, button, "Focus stays on the button during the transition");

await finished.catch(() => {});
assert_equals(document.activeElement, button, "Focus stays on the button after the transition");
}, description);
}
Loading

0 comments on commit 0d46f1c

Please sign in to comment.