SimonPadbury / b4st

A Bootstrap 4 Starter Theme, for WordPress
The Unlicense
311 stars 104 forks source link

Parameters added to the JS scripts are removed #50

Closed delitestudio closed 6 years ago

delitestudio commented 6 years ago

Hello,

first of all thanks for this great starter theme!

I've a problem. I'm loading a Google Maps script in this way:

wp_register_script(
    'googlemaps',
    '//maps.googleapis.com/maps/api/js?key=xxx',
    array(),
    null
);

wp_enqueue_script('googlemaps');

Here is the resulting HTML code:

<script type=‘text/javascript’ src=‘//maps.googleapis.com/maps/api/js’></script>

As you can see, the key parameter has disappeared. After some analysis, I realized that this is due to this code included in the file functions/cleanup.php:

if ( ! function_exists('progenitor_remove_script_version') ) {
  function progenitor_remove_script_version( $src ) {
    $parts = explode( '?', $src );
    return $parts[0];
  }
}
add_filter( 'script_loader_src', 'progenitor_remove_script_version', 15, 1 );
add_filter( 'style_loader_src', 'progenitor_remove_script_version', 15, 1 );

I'm not sure what is its purpose, but it seems a bit too aggressive, what do you think?

SimonPadbury commented 6 years ago

Hi delitestudio,

Feel totally free to reinstate anything in the cleanup. You will not harm anything.

Sorry for the reference to another theme, progenitor. I will change that to b4st -- it was a copy paste from another of my themes, here https://github.com/Progenitor-Theme . I just corrected this in b4st, so take a look at functions/cleanup.php again.

delitestudio commented 6 years ago

Thanks for the answer. If its purpose, as I seem to infer from the function name (remove_script_version), is to remove the version number, can we possibly change it in this way?

function progenitor_remove_script_version( $src ) {
    if ( strpos( $src, 'ver=' ) )
        $src = remove_query_arg( 'ver', $src );
    return $src;
}
SimonPadbury commented 6 years ago

Other people who have asked for improvements to b4st (and its ancestors) over the years, asked for this removal of the script versions. It's because they think hiding this information improves security. I allowed this addition to the theme, as it was a small change and it did no harm, as far as I could see.

But really, it does not really improve security, so I think. Removing all the cleanup scripts will do no harm. They can go.

delitestudio commented 6 years ago

Ok, thanks for your time!

SimonPadbury commented 6 years ago

No problem!