Skip to content

Conversation

@westonruter
Copy link
Owner

No description provided.

gemini-code-assist[bot]

This comment was marked as outdated.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to invalidate pages restored from closed tabs by persisting the bfcache session token in a hidden input field. The changes involve updating JavaScript logic to read and compare this token on pageshow events and modifying PHP to render and pass the necessary data. My review highlights some functional limitations and a UI inconsistency related to the new hidden input field.

Using sessionStorage is more reliable than hoping a browser will persist a value in an input field after back/forward navigation or restoring a closed tab, but it doesn't solve the problem of detecting a regular navigation.
@westonruter westonruter force-pushed the try/closed-tab-restoration branch from 2fafde1 to 4184193 Compare July 2, 2025 23:46
@westonruter

This comment was marked as outdated.

@westonruter

This comment was marked as outdated.

@gemini-code-assist

This comment was marked as outdated.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the bfcache invalidation logic to be more robust by using user session data and introduces a new mechanism using BroadcastChannel to evict pages from bfcache. It also enhances the login screen with a popover to inform users about the instant navigation feature. The changes are well-structured and improve the plugin's functionality. My review includes suggestions to fix a documentation link, align a comment with its implementation, remove an unused import, and improve cookie security.

westonruter and others added 5 commits July 6, 2025 16:07
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@westonruter
Copy link
Owner Author

/gemini review

@westonruter westonruter marked this pull request as ready for review July 7, 2025 00:02
@westonruter westonruter changed the title Try invalidating pages restored from closed tabs Improve bfcache invalidation via BroadcastChannel, store token in user session, promote feature on login Jul 7, 2025
@westonruter
Copy link
Owner Author

/gemini summary

@gemini-code-assist
Copy link
Contributor

Summary of Changes

This pull request enhances the browser Back/Forward Cache (bfcache) handling for authenticated WordPress users, focusing on improved privacy and user experience. It introduces a new invalidation mechanism using BroadcastChannel, improves session token persistence, and promotes the feature to users on the login form.

Highlights

  • Enhanced BFCache Invalidation: Implemented a new BroadcastChannel-based mechanism to evict pages from bfcache when the login screen is accessed. This complements the existing pageshow event-based invalidation, providing a more robust method, especially for Chrome and Firefox.
  • Improved Session Token Persistence: The bfcache session token is now stored within the user's WordPress session data, ensuring persistence even if the cookie is deleted. This allows for better management and re-setting of the cookie when needed.
  • User Feature Promotion: Added a UI element (sparkle icon button) and popover to the login form, next to the 'Remember Me' checkbox, informing users about the 'Instant Back/Forward Navigation' feature.
  • Refined BFCache Logic: Updated the JavaScript handling for pageshow events to compare the initialSessionToken (captured on page load) with the current session token. Added debug logging to clarify when a page is restored from bfcache or invalidated.
Changelog
  • README.md
    • Updated the description of bfcache safeguards to include the new BroadcastChannel mechanism for eviction.
    • Added a new section discussing the privacy implications and current browser behavior for re-opening closed tabs, and the potential future of ClosedTabCache.
  • bfcache-invalidation.js
    • Renamed script to jsonScript for clarity.
    • Introduced bfcacheInvalidatedStorageKey for debug logging in session storage.
    • Updated the data structure to use loginBroadcastChannelName instead of interimLoginBroadcastChannelName.
    • Modified the BroadcastChannel listener to primarily trigger bfcache eviction, removing the latestSessionToken update.
    • Changed the pageshow event handler to use initialSessionToken (captured once on page load) for comparison, and added debug logging for bfcache restoration and invalidation.
  • nocache-bfcache.php
    • Added WP_Session_Tokens to use statements.
    • Renamed INTERIM_LOGIN_BROADCAST_CHANNEL_NAME to LOGIN_BROADCAST_CHANNEL_NAME and updated its description.
    • Introduced BFCACHE_SESSION_TOKEN_USER_SESSION_KEY constant.
    • Added attach_session_information function and hooked it to the attach_session_information filter to store bfcache session tokens in user sessions.
    • Added get_user_bfcache_session_token function to retrieve the token from user sessions.
    • Modified filter_nocache_headers to check for the bfcache session token in the user's session and re-set the cookie if missing.
    • Added set_bfcache_session_token_cookie helper function.
    • Updated set_logged_in_cookie to use the session-stored token and the new cookie setting helper, and adjusted the filter's accepted argument count from 5 to 6.
    • Modified enqueue_script_module to check for the presence of a user's bfcache session token instead of just is_user_logged_in().
    • Updated export_script_module_data to reflect the new broadcast channel name.
    • Renamed print_js_enabled_login_form_field to print_login_form_additions.
    • print_login_form_additions now includes inline CSS, a hidden JavaScript-enabled input, and a new button with a popover to explain the bfcache feature on the login form.
    • Renamed print_interim_login_script to print_login_accessed_broadcast_script and updated its logic to broadcast a message on login screen access.
Activity
  • Requested a summary of the pull request.
  • Prior to these changes, @gemini-code-assist[bot] left a high-priority review comment regarding the enqueue_script_module function's restriction to logged-in users, suggesting it should apply to all no-store pages. While this PR refines the condition, the broader scope limitation remains.
  • Prior to these changes, @gemini-code-assist[bot] left another high-priority review comment concerning the print_bfcache_session_token_field (likely referring to the print_js_enabled_login_form_field function, now print_login_form_additions) and its limitation for non-logged-in users on closed tabs.
  • Prior to these changes, @gemini-code-assist[bot] left a medium-priority review comment pointing out that a 'hidden' input field was being rendered as type="text" and was visible. This specific issue has been addressed in the current changes, as the input is now correctly set to type="hidden".
  • Prior to these changes, @gemini-code-assist[bot] left a high-priority review comment that the secure flag for setcookie is hardcoded to false.
  • Prior to these changes, @gemini-code-assist[bot] left a medium-priority review comment that the JSDoc @see link on this line uses an incorrect closing bracket.
  • Prior to these changes, @gemini-code-assist[bot] left a medium-priority review comment that the JSDoc comment for onPageShow indicates it handles re-opening closed tabs, but the implementation relies on event.persisted, which is true only for bfcache restorations.
  • Prior to these changes, @gemini-code-assist[bot] left a medium-priority review comment that the WP_Admin_Bar class is imported but appears to be unused in this file.

@westonruter westonruter merged commit dc145af into main Jul 7, 2025
1 check passed
@westonruter westonruter deleted the try/closed-tab-restoration branch July 16, 2025 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants