Versioning is being forced for JavaScript and CSS assets with no override

[amattie]

It seems that there's currently no way to perform script and css resource versioning manually with wp_enqueue_script and wp_enqueue_style. When false, null, an empty string, or anything that evaluates to false is passed as the version argument, the version of WordPress is appended to the asset.

This causes problems when trying to use wp_enqueue_script or wp_enqueue_style to manage assets on external web servers where something that is interpreted as a query string on a "normal" web server is interpreted as part of the asset name on that "special" web server. In my case, I'm trying to reference a script placed on Amazon S3 and served through their CloudFront CDN. I think a case can also be made where you don't want versioning on assets that you don't fully control. A good example of this is Google Maps, which I'm also using. Passing ?ver=2.9 to the Google Maps JavaScript loader could have unknown consequences in the future.

My proposal is to skip the setting of the version in the query string for the asset if an empty string is passed in for that argument. The way I did this in my patch was by simply implementing strict type checking in a few key spots. This change leaves the default functionality in place while still providing a way for developers to override the version setting functionality.

[amattie]

Patch for new functionality of wp_enqueue_*

[scribu]

+1 on this: shouldn't force version argument on users.

That being said, I don't get what the purpose of the following code from your patch is:

 	235	                if ( $this->ver === false ) 
236	236	                        $this->ver = false; 

:)

[Denis-de-Bernardy]

agreeing with scribu. the idea is good, but the patch is extremely fishy.

[dd32]

That !=== false is required as if you pass a empty string, PHP will interprate that as false. ( == false && 0 == false & null == false && array() == false && false == false) == true

[dd32]

Damn formatter..

 ('' == false && 0 == false & null == false && array() == false && false == false) == true 

[lloydbudd]

Be sure to update the php doc... and ​http://codex.wordpress.org/Function_Reference/wp_enqueue_script is already incorrect.

[scribu]

Remove default version altogether

[scribu]

Add default version only for null value

[scribu]

Replying to dd32:

Damn formatter..

 ('' == false && 0 == false & null == false && array() == false && false == false) == true 

No, I mean what's the point of checking for false, and then assigning false to the same variable?

I've added two patches:

​11315_remove.diff removes the default version altogether, since it isn't that useful, IMO

​11315_override.diff corrects the patch from amattie and includes the default version only when not explicitly set by the user.

[scribu]

This is what I've tested the patches agains:

add_action('template_redirect', 'debug_script_loader');
function debug_script_loader() {
	wp_enqueue_script('test1', 'example.com', array());
	wp_enqueue_script('test2', 'example.com', array(), 1.2);
	wp_enqueue_script('test3', 'example.com', array(), false);

	wp_enqueue_style('test1', 'example.com', array());
	wp_enqueue_style('test2', 'example.com', array(), 1.2);
	wp_enqueue_style('test3', 'example.com', array(), false);
}

[dd32]

No, I mean what's the point of checking for false, and then assigning false to the same variable?

Sorry.. I thought one of them was $ver, not $this->ver :(

[scribu]

Replying to dd32:

No, I mean what's the point of checking for false, and then assigning false to the same variable?

Sorry.. I thought one of them was $ver, not $this->ver :(

No worries. :)

[lloydbudd]

Replying to scribu:

​11315_remove.diff removes the default version altogether, since it isn't that useful, IMO

It's quite useful. It breaks the browser cache when you upgrade WordPress, so your site's visitors and bloggers are not served a hodge hodge . Debugging nightmare.

​11315_override.diff corrects the patch from amattie and includes the default version only when not explicitly set by the user.

Sounds good... still no documentation of the behavior -- removed 'has-patch' pending doc.

[amattie]

Regarding the purpose of this:

 	235	                if ( $this->ver === false ) 
236	236	                        $this->ver = false; 

You're correct in that that's kinda useless, but perhaps 235-236 should be removed altogether then. Without removing it (as it is in 11315_override.diff or 11315_remove.diff), $this->ver is set to false if an empty string is passed in since evals to false. If $this->ver is set to false (strict) at that point, checking for === false in the other spots will become useless.

In other words, the suggested behavior of setting the value to an empty string to override the versioning will have no effect if 235-236 is left in there.

Thoughts on removing 235-236 in _WP_Dependency?

[amattie]

Override versioning with empty string, strict false assignment removed in _WP_Dependency

[scribu]

Replying to amattie:

Regarding the purpose of this:

 	235	                if ( $this->ver === false ) 
236	236	                        $this->ver = false; 

You're correct in that that's kinda useless, but perhaps 235-236 should be removed altogether then. Without removing it (as it is in 11315_override.diff or 11315_remove.diff), $this->ver is set to false if an empty string is passed in since evals to false.

But they are removed altogether in both patches.

[amattie]

Replying to scribu:

But they are removed altogether in both patches.

Argh, my apologies -- I totally missed that.

[scribu]

Add documentation, use empty string instead of null

[mdawaffe]

+1

[azaozz]

(In [12558]) WP_Dependencies: pass NULL to disable script and style version query strings, props scribu amattie, fixes #11315

[azaozz]

Don't see a good reason to change the current behaviour. Passing FALSE should still append the default version, pass NULL to disable it.

[azaozz]

(In [12559]) Too many question marks, see #11315

[F J Kaiser]

+1