mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-07-31 01:36:15 -04:00
webapi: same-fragment navigations must not reload the page
After rebasing onto main, WPT /dom/events/Event-dispatch-single-activation-behavior.html went from 3/132 to 0/0 (harness never reported): the test repeatedly clicks `<a href="#link">` anchors and resets with `location.hash = ""`, and both patterns ended up scheduling full same-URL reloads, putting the page in a reload loop. As with the previous commit, the pre-rebase code masked this because queued navigations rarely executed; the HttpClient refactor made them reliable. Two fixes, per the HTML navigate steps (a navigation is a fragment navigation when the URL equals the document's URL excluding fragments and its fragment is non-null) and the Location hash setter (if the fragment doesn't change, no navigation happens): - Frame.scheduleNavigationWithArena: re-navigating to the exact current URL is only a reload when the URL has no fragment. With a fragment (e.g. re-clicking the same `#link` anchor) it's now a no-op fragment navigation: no reload, and since the fragment didn't change, no hashchange and no history entry. Upstream's "identical URL reloads" behavior (iframe.src = "about:blank" twice) is untouched — those URLs carry no fragment. - Location.setHash: skip the navigation entirely when the normalized fragment equals the current one, so `location.hash = ""` on a fragment-less URL (and re-assigning the current fragment) no longer schedules a same-URL reload. Assigning "" still strips the fragment from the URL, matching the existing tests/window/location.html expectations. Unit test added in tests/window/location.html: href stays stable across repeated `hash = ""` and same-fragment re-assignments. Coverage: /dom/events/Event-dispatch-single-activation-behavior.html 0/0 -> 83/132 (3/132 before the rebase; the gain past 3 comes from the handler-return-value fix in the previous commit).
This commit is contained in:
committed by
Karl Seguin
parent
daa3b1d451
commit
027936ef90
@@ -894,6 +894,20 @@ fn scheduleNavigationWithArena(originator: *Frame, arena: Allocator, request_url
|
||||
};
|
||||
|
||||
const session = target._session;
|
||||
|
||||
// Re-navigating to the exact current URL is only a reload when the URL
|
||||
// has no fragment. With a fragment it's a fragment navigation per the
|
||||
// HTML "navigate" steps (url equals the document's URL excluding
|
||||
// fragments and url's fragment is non-null): no reload, and since the
|
||||
// fragment didn't change, no hashchange and no new history entry either.
|
||||
if (!opts.force and
|
||||
std.mem.eql(u8, target.url, resolved_url) and
|
||||
std.mem.indexOfScalar(u8, resolved_url, '#') != null)
|
||||
{
|
||||
session.releaseArena(arena);
|
||||
return;
|
||||
}
|
||||
|
||||
// Short-circuit only true fragment-only navigations (same path/query, different
|
||||
// fragment). Identical URLs fall through and trigger a real reload.
|
||||
const is_fragment_navigation = !std.mem.eql(u8, target.url, resolved_url) and URL.eqlDocument(target.url, resolved_url);
|
||||
|
||||
@@ -68,3 +68,24 @@
|
||||
location.hash = "";
|
||||
testing.expectEqual("", location.hash);
|
||||
</script>
|
||||
|
||||
<script id=location_hash_no_change>
|
||||
// Per the Location hash setter, assigning a value that doesn't change the
|
||||
// fragment must not navigate at all: re-assigning "" on a fragment-less URL
|
||||
// must not schedule a same-URL reload, and re-assigning the current
|
||||
// fragment must be a no-op too.
|
||||
const stable_href = location.href;
|
||||
location.hash = "";
|
||||
location.hash = "";
|
||||
testing.expectEqual(stable_href, location.href);
|
||||
|
||||
location.hash = "#same";
|
||||
const with_fragment = location.href;
|
||||
location.hash = "#same";
|
||||
location.hash = "same";
|
||||
testing.expectEqual(with_fragment, location.href);
|
||||
testing.expectEqual("#same", location.hash);
|
||||
|
||||
location.hash = "";
|
||||
testing.expectEqual(stable_href, location.href);
|
||||
</script>
|
||||
|
||||
@@ -101,21 +101,30 @@ pub fn setSearch(_: *const Location, search: []const u8, frame: *Frame) !void {
|
||||
}
|
||||
|
||||
pub fn setHash(_: *const Location, hash: []const u8, frame: *Frame) !void {
|
||||
const normalized_hash = blk: {
|
||||
if (hash.len == 0) {
|
||||
const old_url = frame.url;
|
||||
const old_url = frame.url;
|
||||
const base_end = std.mem.indexOfScalar(u8, old_url, '#') orelse old_url.len;
|
||||
// Includes the leading '#'; empty when the URL has no fragment.
|
||||
const old_fragment = old_url[base_end..];
|
||||
|
||||
break :blk if (std.mem.indexOfScalar(u8, old_url, '#')) |index|
|
||||
old_url[0..index]
|
||||
else
|
||||
old_url;
|
||||
} else if (hash[0] == '#')
|
||||
break :blk hash
|
||||
else
|
||||
break :blk try std.fmt.allocPrint(frame.call_arena, "#{s}", .{hash});
|
||||
const normalized_hash: []const u8 = blk: {
|
||||
if (hash.len == 0) {
|
||||
break :blk "";
|
||||
} else if (hash[0] == '#') {
|
||||
break :blk hash;
|
||||
}
|
||||
break :blk try std.fmt.allocPrint(frame.call_arena, "#{s}", .{hash});
|
||||
};
|
||||
|
||||
return frame.scheduleNavigation(normalized_hash, .{
|
||||
// Per the Location hash setter, when the fragment doesn't change no
|
||||
// navigation happens at all — in particular `location.hash = ""` on a
|
||||
// fragment-less URL must not turn into a same-URL reload.
|
||||
if (std.mem.eql(u8, old_fragment, normalized_hash)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const target_url = if (normalized_hash.len == 0) old_url[0..base_end] else normalized_hash;
|
||||
|
||||
return frame.scheduleNavigation(target_url, .{
|
||||
.reason = .script,
|
||||
.kind = .{ .replace = null },
|
||||
}, .{ .script = frame });
|
||||
|
||||
Reference in New Issue
Block a user