Problem/Motivation
MasqueradeBlock::build() renders MasqueradeForm inline. Since #2559011, FormBuilder::prepareForm() gives every form rendered for an authenticated user a form token element with max-age: 0 unless the form opts in to caching. That max-age bubbles into the block and then into the page, so every page that contains the Masquerade block is uncacheable in the dynamic page cache for any user who has a masquerade permission (X-Drupal-Dynamic-Cache: UNCACHEABLE, X-Drupal-Cache-Max-Age: 0).
#2900413 already renders the switch-back link through a placeholder, but the form branch of build() still carries the token's max-age.
Steps to reproduce
- Enable
dynamic_page_cacheandbig_pipe, place the Masquerade block in any region. - Log in as a user with "Masquerade as any user" (or any other masquerade permission).
- Request any page twice and inspect the response headers:
X-Drupal-Dynamic-CachestaysUNCACHEABLEandX-Drupal-Cache-Max-Ageis0on both requests. - Remove the block: the second request is a dynamic page cache
HIT.
Proposed resolution
Since Drupal 11.1 a form can opt in to render caching by setting a top-level #cache max-age (change record, #2578855). Core then no longer forces the token element to max-age: 0; the CSRF token itself is already rendered through a placeholder. Nothing in the masquerade form varies per user, so set $form['#cache']['max-age'] = Cache::PERMANENT in MasqueradeForm::buildForm().
On Drupal 10 the token element is hard-coded to max-age: 0, so the change is a no-op there. Drupal 10 is end of life, so this issue does not try to cover it; a Drupal 10 compatible alternative would be to render the form through a #lazy_builder placeholder like the switch-back link.
Once #3395157 makes forms cacheable by default, the explicit opt-in becomes unnecessary.
Remaining tasks
Review.
API changes
None.
Issue fork masquerade-3630613
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
moshe weitzman commentedComment #5
mortona2k commentedAdded a test, which discovered that the page cache was already a HIT, but now the block render cache works and the response is no longer max-age: 0.
Added to #3625731: Combined branch, next release?.
Needs review for test.
Comment #6
moshe weitzman commented> Added a test, which discovered that the page cache was already a HIT, but now
the block render cache works and the response is no longer max-age: 0.
How can page cache be a hit AND the response to be max-age 0?
Comment #7
mortona2k commentedSorry, I meant to say dynamic page cache. It's just a nit with the summary.
Claude says:
It was already a HIT: the token's max-age 0 makes the block get auto-placeholdered, so DPC caches the page around it, but the block is then rebuilt on every request and its max-age 0 ends up in the response. So the summary's "uncacheable in the dynamic page cache" isn't quite what happens. What the MR fixes is the block render cache (no more form build per request) and the max-age: 0 on the response, and the test asserts both.Comment #8
mortona2k commentedIf the test is not needed, I can back it out and approve your changes.
Comment #9
moshe weitzman commentedI personally think the test exercises core's code, not masquerade. I dont feel strongly though.
Comment #10
mortona2k commentedBack to how you had it.