Fix compatibility with the Two-Factor plugin and any plugins with interstitial login screens #7
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @westonruter, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses compatibility issues with the Two-Factor plugin and other interstitial login screens by implementing a robust back/forward cache (bfcache) management system. The changes allow WordPress to leverage browser bfcache for faster navigation for logged-in users who opt-in, while ensuring that sensitive session data is not improperly cached and that pages are correctly invalidated upon logout or user changes. The solution involves both server-side header manipulation and client-side JavaScript for session integrity.
Highlights
- Enhanced Back/Forward Cache (BFCache) Management: Introduced a comprehensive system to manage browser back/forward cache for logged-in users. This allows pages to be stored in bfcache for instant navigation while ensuring user session integrity and privacy, particularly when using 'Remember Me' and JavaScript is enabled.
- Login Screen Opt-in and Information: Added a new user interface element to the login screen, including a popover, to inform users about the 'Instant Back/Forward Navigation' feature and allow them to opt-in by checking 'Remember Me' when JavaScript is active.
- Dynamic Cache-Control Header Modification: Implemented logic to dynamically modify the
Cache-ControlHTTP header. For opted-in users, theno-storedirective is removed and replaced withprivate,no-cache,max-age=0, andmust-revalidateto allow bfcache while preventing proxy caching. - Client-Side BFCache Invalidation: Introduced JavaScript modules for client-side bfcache invalidation. One module uses the
pageshowevent (primarily for Safari) to reload pages if the session token changes (e.g., after logout or user switch). Another module attempts to useBroadcastChannel(for Chrome/Firefox) to evict pages from bfcache when an unauthenticated user lands on the login screen, though aTODOcomment notes this is currently not working as expected. - Code Modularization and Structure: Refactored the plugin's codebase by separating concerns into new dedicated files:
bfcache-opt-in.phpfor opt-in logic,bfcache-invalidation.phpfor invalidation logic, andscript-loader.phpfor centralized script and style registration. This significantly improves maintainability and organization.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request refactors the plugin to improve its structure and maintainability, using a cookie instead of a hidden form field for detecting JavaScript. There's a critical issue with cookie expiration calculations and a concern about the reliability of the BroadcastChannel invalidation mechanism.
| * Chrome and Firefox. This behavior has been [proposed](https://github.com/whatwg/html/issues/7253#issuecomment-2632953500) | ||
| * for standardization. | ||
| * | ||
| * TODO: In previous testing this seemed to work, but now it is not working in Chrome or Firefox either, puzzlingly. |
There was a problem hiding this comment.
The TODO comment indicates that the BroadcastChannel approach may not be working as expected in Chrome or Firefox. Since this is a key part of the bfcache invalidation strategy, its potential unreliability is a concern. Please provide more details on the testing that led to this conclusion.
There was a problem hiding this comment.
@gemini-code-assist I will address this in a subsequent PR.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a beneficial refactoring, organizing the JavaScript and PHP code into a more maintainable structure. The changes improve compatibility with interstitial login screens. The review highlights TODO comments that suggest some functionality may not be working as intended; addressing these points will ensure the code is robust and clear.
| * Chrome and Firefox. This behavior has been [proposed](https://github.com/whatwg/html/issues/7253#issuecomment-2632953500) | ||
| * for standardization. | ||
| * | ||
| * TODO: In previous testing this seemed to work, but now it is not working in Chrome or Firefox either, puzzlingly. |
There was a problem hiding this comment.
The TODO comment here raises a significant concern that this script's core functionality is not working as expected. Shipping code that is known to be non-functional can lead to maintenance challenges and unpredictable behavior. It's recommended to either fix the implementation to ensure it works reliably across the intended browsers, or remove the script and its corresponding enqueue logic from includes/bfcache-invalidation.php if this approach is no longer considered viable.
There was a problem hiding this comment.
To be addressed in follow-up PR.
| VERSION | ||
| ); | ||
|
|
||
| // This is used by Chrome and Firefox. TODO: Actually, this doesn't seem to be the case! |
There was a problem hiding this comment.
The TODO comment indicates uncertainty about which browsers use this script. It's crucial that the purpose and necessity of each part of the code are clear. Please investigate and clarify this. If the script is indeed necessary, the comment should be updated to reflect its confirmed purpose. If it's found to be redundant, it would be best to remove it and its registration to avoid dead code.
There was a problem hiding this comment.
To be addressed in follow-up PR.
See WordPress/two-factor#705
JS and PHP code is also better organized by putting JS into static files and splitting up PHP files.