1353 bulk editor guard unsaved edits against hard navigation paths#23449
Merged
JorPV merged 10 commits intoJul 14, 2026
Merged
Conversation
Extends requestSwitch/commitSwitch with a "navigate" kind whose target is a URL, so a full page navigation can be deferred to the same confirmation flow as tab and content-type switches. The navigate kind skips the no-op current-view check and, on commit, hands off to window.location instead of mutating view state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… guard The Yoast logo and "Back to Tools" links rendered plain anchors that navigated away and silently dropped pending edits. Their clicks now dispatch a guarded navigate switch, deferring to the same confirmation modal (and Premium's pending-AI slot) as the tab and content-type switches. Modified clicks (open in new tab or window) still pass through to the browser unguarded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refresh, tab close, and the browser back button leave the page without touching the in-app links, so the styled modal cannot catch them. A beforeunload listener now triggers the browser's native confirm dialog whenever there are unsaved inline edits or an external plugin (Premium AI) reports pending changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… away Switching content type, then trying to switch back to the earlier type did nothing when the earlier switch left the stored active name resolved from empty. The content-type click handler closed over the resolved active id, and the sidebar navigation freezes the first handler it is given, so the comparison ran against a stale active id and treated the switch-back as a no-op. The handler now reads the active id from a ref, staying referentially stable while comparing against the current value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FAMarfuaty
added
changelog: non-user-facing
…ouble prompt The beforeunload guard also read the external pending-changes flag (Premium AI suggestions). Confirming Premium's own modal to leave via an in-app link then fired the browser's native prompt on top of it, and the navigate commit left the deferred switch in place so the self-heal effect re-fired the navigation even after the user chose to stay. The guard now arms on Free's own inline edits only — Premium owns the unload guard for its state — and the navigate commit clears the deferred switch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coverage Report for CI Build 8Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 46.112%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the bulk editor’s existing pending-changes guard to cover “hard” navigation paths that leave the page (Yoast logo, “Back to Tools”, refresh/close/back via beforeunload), and fixes a regression where switching back to the initially selected content type could be ignored due to a stale click handler.
Changes:
- Adds a new guarded switch kind (
navigate) that routes hard navigations through the existing pending-switch / modal flow. - Arms a native
beforeunloadprompt only for Free’s own unsaved inline edits (not external/premium pending changes). - Fixes content-type switch-back by reading the active content type id from a ref to avoid stale handler closures.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/js/src/bulk-editor/store/pending-switch.js | Adds navigate switch kind and commit behavior to redirect via window.location.href, reusing existing guard logic. |
| packages/js/src/bulk-editor/app.js | Adds useBeforeUnload for unsaved inline edits; intercepts logo/Back-to-Tools clicks to request guarded navigation; fixes stale content-type handler using a ref. |
| packages/js/src/bulk-editor/components/bulk-editor-nav.js | Routes Yoast logo clicks through a new onNavigate prop so hard navigation can be guarded. |
| packages/js/src/bulk-editor/components/back-to-tools-link.js | Adds onNavigate support so “Back to Tools” click can be intercepted/guarded. |
| packages/js/tests/bulk-editor/store/pending-switch.test.js | Adds unit tests covering navigate request/deferral and navigation commit behavior. |
| packages/js/tests/bulk-editor/bulk-editor-nav.test.js | Adds tests ensuring logo and Back-to-Tools clicks are routed through onNavigate with correct hrefs. |
| packages/js/tests/bulk-editor/app.test.js | Adds integration-style tests for switch-back regression and hard-navigation guarding / beforeunload arming behavior. |
…n to prevent open redirects
JorPV
reviewed
Jul 14, 2026
JorPV
reviewed
Jul 14, 2026
| dispatch.clearPendingSwitch(); | ||
| // Security: target flows unvalidated into location.href, so it must stay a server-generated URL (the localized | ||
| // links in bulk-editor-integration.php). If a link ever derives from request input, validate it here first | ||
| // (same-origin and an http(s) scheme) to avoid an open redirect or a javascript: URI. |
Contributor
There was a problem hiding this comment.
Suggested change
| // (same-origin and an http(s) scheme) to avoid an open redirect or a javascript: URI. | |
| // Always validate server-generated URLs before using them in location.href to prevent open redirects or malicious URIs. |
| * pending changes: the switch is then held until the user resolves it, otherwise it commits immediately. | ||
| * pending changes: the switch is then held until the user resolves it, otherwise it commits immediately. A | ||
| * "navigate" switch targets a URL rather than a view value, so it skips the no-op check that compares against the | ||
| * current view. |
Contributor
There was a problem hiding this comment.
Suggested change
| * current view. | |
| * Switch requests are blocked during manual edits or pending plugin changes; otherwise, they commit immediately. A 'navigate' switch targets a URL, bypassing the current view check. |
| const isPremium = useSelect( ( select ) => select( STORE_NAME ).selectPreference( "isPremium", false ), [] ); | ||
| // Only Free's own unsaved inline edits arm the native unload guard. An external plugin's pending changes | ||
| // (Premium's AI suggestions) are guarded by that plugin's own modal, and it owns any unload guard for its state; | ||
| // reading its flag here would double-prompt on a navigation the user has already confirmed through that modal. |
Contributor
There was a problem hiding this comment.
Suggested change
| // reading its flag here would double-prompt on a navigation the user has already confirmed through that modal. | |
| // Free's unsaved inline edits trigger the native unload guard; Premium handles its own pending changes and unload prompts. |
|
|
||
| // The resolved active id is read from a ref so the handler below can stay referentially stable: the sidebar | ||
| // navigation freezes the first click handler it receives, so a handler closing over activeContentTypeId would | ||
| // keep comparing against a stale value and silently drop a switch back to an earlier content type. |
Contributor
There was a problem hiding this comment.
Suggested change
| // keep comparing against a stale value and silently drop a switch back to an earlier content type. | |
| // The active ID is read from a ref to keep the click handler referentially stable, preventing stale comparisons that could silently drop a switch to an earlier content type. |
Co-authored-by: Jordi-PV <88150345+JorPV@users.noreply.github.com>
…ate and parameters
Contributor
|
CR + ACC ✅ |
JorPV
deleted the
1353-bulk-editor-guard-unsaved-edits-against-hard-navigation-paths
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Follow-up to #23447, which guarded unsaved inline edits and pending AI suggestions only for the two in-app view switches (appearance tabs and content types). Paths that leave the page entirely still dropped edits silently: the Yoast logo link, the "Back to Tools" link, and browser refresh / tab close / back button. This PR guards those.
It also fixes a switch-back regression from #23447: the content-type click handler compared against a stale active id, so returning to the initially selected content type did nothing.
Summary
This PR can be summarized in the following changelog entries:
Relevant technical choices:
requestSwitch/commitSwitchguard via a newnavigatekind, so the styled modal, the self-heal effect, and Premium's pending-changes slot all apply unchanged (including for pending AI suggestions).beforeunloadlistener, since the browser's native dialog can't route through the styled modal. It arms on Free's own inline edits only: an external plugin's pending changes (Premium AI) are guarded by that plugin's own modal, and Premium owns any unload guard for its state — reading its flag here would fire a native prompt on top of a navigation the user already confirmed.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Hard-navigation links — manual edits (Free):
Refresh / tab close / browser back (Free):
In-app view switches still guarded (regression check):
Content-type switch-back:
Pending AI suggestions (Premium — requires the Premium counterpart on branch
1353-bulk-editor-guard-unsaved-edits-against-hard-navigation-paths, Premium active with a valid subscription):Note
Accompanying PR: https://github.com/Yoast/wordpress-seo-premium/pull/5018
Relevant test scenarios
The
beforeunloaddialog is browser-native, so its wording and behaviour vary per browser. In Free, the native refresh/close/back guard covers manual edits only; the pending-AI-suggestions equivalent lives in the Premium counterpart branch referenced above, which the Premium test steps require.Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Other environments
Documentation
Quality assurance
grunt build:imagesand committed the results, if my PR introduces or edits images or SVGs.Innovation
innovationlabel.Fixes Yoast/reserved-tasks#1353