Retain CCNS when the user has JavaScript disabled #4
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 focuses on improving the handling of browser back/forward cache (bfcache) for authenticated users, specifically addressing scenarios where JavaScript is disabled. My changes ensure that sensitive pages are not inadvertently cached in bfcache when JavaScript isn't available to manage invalidation, thereby enhancing user privacy and security. This is achieved by making the bfcache session token cookie conditional on JavaScript detection during login, which in turn controls the Cache-Control: no-store header.
Highlights
- JavaScript Detection for Login: I've introduced a mechanism to detect if JavaScript is enabled during the login process. This is done by dynamically adding a hidden input field to the login form via a new script module, which will only be present in the POST data if JavaScript executes.
- Conditional BFcache Cookie Setting: I've modified the
set_logged_in_cookiefunction to only set the bfcache session token cookie if JavaScript is detected as enabled during login. If JavaScript is disabled, this cookie will not be set. - Cache-Control Header Logic for JS Disabled Users: I've updated the
filter_nocache_headersfunction. It will now retain theno-storedirective in theCache-Controlheader if the bfcache session token cookie is not present. This ensures authenticated pages are not cached in bfcache when JavaScript is disabled, preserving privacy. - Code Refactoring for Inline Scripts: I've added a new helper function,
print_inline_script_module_from_html_script_tag, to standardize and improve the process of printing inline script modules. This ensures proper handling and compatibility with Content Security Policy (CSP) and other filters.
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 introduces a mechanism to retain the Cache-Control: no-store header for users with JavaScript disabled. The implementation correctly uses a hidden form field added via JavaScript during login to detect JS support and conditionally sets a session cookie. This cookie then determines whether to modify the cache headers. I've added a couple of suggestions to improve the robustness of the new JavaScript and PHP helper functions.
| input.type = 'hidden'; | ||
| input.name = <?php echo wp_json_encode( JAVASCRIPT_ENABLED_INPUT_FIELD_NAME ); ?>; | ||
| input.value = '1'; | ||
| document.getElementById( 'loginform' ).appendChild( input ); |
There was a problem hiding this comment.
Consider adding a check to ensure the loginform element exists before appending the input. This prevents potential JavaScript errors if the form ID is different or the element is not present.
document.getElementById( 'loginform' )?.appendChild( input );There was a problem hiding this comment.
@gemini-code-assist The login form should be guaranteed to be present in this context. If not, then I actually would want there to be an error to be able to notice something is wrong.
There was a problem hiding this comment.
Thank you for clarifying. Your approach to intentionally surface an error if the loginform element is not present is a valid design decision, especially in an environment where its presence is guaranteed and a missing element would indicate a deeper issue that needs immediate attention. I understand and agree with this perspective for this context.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request enhances the plugin to retain CCNS when JavaScript is disabled, improving privacy. A JavaScript-injected form field is used to detect client capabilities. The logic is well-contained, and the refactoring of inline script printing improves code clarity. A suggestion is provided to enhance the robustness of the client-side code.
No description provided.