Skip to content

feat: expose shouldSkipOnBackForwardUI on NavigationEntry - #51961

Open
MarshallOfSound wants to merge 2 commits into
mainfrom
feat/navigation-entry-skippable
Open

feat: expose shouldSkipOnBackForwardUI on NavigationEntry#51961
MarshallOfSound wants to merge 2 commits into
mainfrom
feat/navigation-entry-skippable

Conversation

@MarshallOfSound

Copy link
Copy Markdown
Member

Description of Change

Chromium's history manipulation intervention marks a history entry "skippable" when the page created it and then navigated away without a user gesture (client-side redirectors, login bounces, pushState back-traps). The native goBack()/goForward()/canGoBack() honor the flag and skip such entries; goToIndex() deliberately does not. The flag lives internally on content::NavigationEntryImpl, and Electron's NavigationEntry structures don't expose it.

This PR exposes the flag as a read-only boolean property — shouldSkipOnBackForwardUI — on the NavigationEntry structures returned by navigationHistory.getEntryAtIndex() and navigationHistory.getAllEntries(). The property name mirrors the internal Chromium name and is open to API-WG bikeshedding.

Motivating use case: an embedder that needs deterministic, policy-gated history navigation peeks the target entry (navigationHistory.getEntryAtIndex(active ± 1)), validates it against a security policy, then commits with goToIndex() so the gated URL is exactly what commits. Without the skippable flag, that pattern can't honor the intervention — on a stack [A, R, B] where R auto-redirects forward, "back" from B lands on R and bounces, while native goBack() would have skipped to A. Exposing the flag lets the embedder walk the peek past skippable entries using Chromium's own ground truth instead of re-implementing the heuristic.

Implementation:

  • A minimal Chromium patch (feat_expose_shouldskiponbackforwardui_on_content_navigationentry.patch) adds a public const accessor ShouldSkipOnBackForwardUI() to content::NavigationEntry, implemented by NavigationEntryImpl which already owns the flag. This is a reasonable candidate to upstream to Chromium proper, as noted in the patch header.
  • The Electron-side change is a single dict.Set() in the content::NavigationEntry* → V8 converter, which backs all NavigationEntry-structure-producing APIs.
  • The property is documented as optional so the TypeScript change stays additive (NavigationEntry is also the input type of navigationHistory.restore(), where the property is ignored).
  • Specs build a real [A, R, B] stack against the spec HTTP server where R navigates away via renderer-initiated location.assign with no user gesture, and assert the flag is [false, true, false] plus the existing behavior contrast: goBack() from B commits A while getEntryAtIndex(active - 1) still returns R.

Checklist

Release Notes

Notes: Added shouldSkipOnBackForwardUI to the NavigationEntry structure, exposing whether Chromium's history manipulation intervention will skip the entry on back/forward navigations.

@MarshallOfSound
MarshallOfSound requested a review from a team as a code owner June 10, 2026 21:16
@MarshallOfSound MarshallOfSound added semver/minor backwards-compatible functionality api-review/requested 🗳 labels Jun 10, 2026
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Jun 10, 2026
@MarshallOfSound MarshallOfSound added target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels Jun 10, 2026

@jkleinsc jkleinsc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM

@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Jun 17, 2026

@itsananderson itsananderson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestion for the explanation of the "optional" property, but otherwise API LGTM

Comment on lines +13 to +14
with `navigationHistory.goToIndex(index)`. Always present on entries returned by Electron; it is
ignored when passed to `navigationHistory.restore()`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this wording will cause confusion. Here's a suggestion that's a little more verbose, but hopefully less likely to cause confusion?

Suggested change
with `navigationHistory.goToIndex(index)`. Always present on entries returned by Electron; it is
ignored when passed to `navigationHistory.restore()`.
with `navigationHistory.goToIndex(index)`. This property is always defined as `true`/`false`
on entries returned by Electron; it is marked as optional because it is ignored when passed
to `navigationHistory.restore()`.

Comment thread docs/api/structures/navigation-entry.md
@github-actions github-actions Bot added the target/44-x-y PR should also be added to the "44-x-y" branch. label Jul 1, 2026
Chromium's history manipulation intervention marks a history entry
skippable when the page created it and navigated away without a user
gesture. goBack()/goForward()/canGoBack() honor the flag; goToIndex()
does not. Expose the flag as a read-only boolean on the NavigationEntry
structures returned by navigationHistory.getEntryAtIndex() and
navigationHistory.getAllEntries() so apps that drive history navigation
through index-based APIs can honor the intervention using Chromium's
own ground truth.

Adds a minimal Chromium patch exposing a public const accessor on
content::NavigationEntry, since the flag previously lived only on
content::NavigationEntryImpl.
@MarshallOfSound
MarshallOfSound force-pushed the feat/navigation-entry-skippable branch from e2465df to f3e2b93 Compare July 27, 2026 23:30
@MarshallOfSound
MarshallOfSound enabled auto-merge (squash) July 27, 2026 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-review/approved ✅ semver/minor backwards-compatible functionality target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. target/44-x-y PR should also be added to the "44-x-y" branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants