[wp-trac] [WordPress Trac] #55491: Replace `unload` event handlers from core

[WordPress Trac]

#55491: Replace `unload` event handlers from core
-------------------------------------+-------------------------------------
 Reporter:  shawfactor               |       Owner:  westonruter
     Type:  enhancement              |      Status:  accepted
 Priority:  normal                   |   Milestone:  6.4
Component:  Administration           |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  good-first-bug has-      |     Focuses:  javascript,
  patch                              |  performance
-------------------------------------+-------------------------------------

Comment (by westonruter):

 Note: Even with the elimination of `unload` event handlers, this may still
 not enable the use of bfcache because admin pages are served with `Cache-
 Control: no-store`. The Lighthouse audit notes this as being a "not
 actionable" failure type. The use of `no-store` can be
 [https://github.com/WordPress/wordpress-
 develop/blob/2cdaf29678b2a978b6fb9a730b051237cf40d3b4/src/wp-
 includes/functions.php#L1494-L1497 found] in `wp_get_nocache_headers()`
 and notably `no-store, private` was just added for logged-in users as of
 WordPress 6.3, specifically in r55968 to fix #21938/#57627:

 > The intention behind this change is to prevent sensitive data in
 responses for logged in users being cached and available to others, for
 example via the browser history after the user logs out.

 Therefore, it seems actually that the disabling of bfcache might be
 considered a new security/privacy feature. Eliminating `unload` handlers
 unfortunately will yield no performance improvement, unfortunately.
 However, since `unload` is being deprecated it is still good to remove its
 use and also to remove noise when doing audits.

 Nevertheless, I think we should consider revisiting #21938 to remove `no-
 store` by default because its presence disables bfcache for all logged-in
 page loads, even on the frontend. The introduction of `no-store` didn't
 cause a performance regression in #21938 because bfcache was already
 disabled due to `unload` event handlers being on the page. But with those
 removed, now bfcache is disabled due to `Cache-Control: no-store` instead.
 It could be that for the 80% of users it is better to ''not'' disable
 bfcache and to instead let users opt-in to that via a plugin if they are
 concerned about privacy/security, such as when they are using a shared
 computer.

 Here's plugin code to remove the `no-store` directive from `Cache-Control`
 header:

 {{{#!php
 <?php
 /**
  * Plugin Name: Disable Cache-Control: no-store
  * Author: Weston Ruter
  */

 add_filter(
         'nocache_headers',
         static function ( $headers ) {
                 if (
                         isset( $headers['Cache-Control'] ) &&
                         str_contains( $headers['Cache-Control'], 'no-
 store' )
                 ) {
                         $directives = array_diff(
                                 preg_split( '/\s*,\s*/', $headers['Cache-
 Control'] ),
                                 array( 'no-store' )
                         );
                         $headers['Cache-Control'] = implode( ', ',
 $directives );
                 }
                 return $headers;
         }
 );
 }}}

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/55491#comment:14>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform