Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#24834 closed defect (bug) (invalid)

wp_enqueue_script passing null for $ver produces unexpected results

Reported by: bobbingwide's profile bobbingwide Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5.2
Component: General Keywords: close
Focuses: Cc:

Description

This is a follow on to #16699.
The documentation states that you can pass a null parameter as the version to prevent any version number being printed.
See source (http://core.trac.wordpress.org/browser/trunk/wp-includes/functions.wp-scripts.php#L37 ) and codex (http://codex.wordpress.org/Function_Reference/wp_enqueue_script )

In my experience passing null still produces a ver= query string
which includes the original query string.
There are also some unexpected changes to the original $src

$src = "http://www.example.com/widget.js?token=something%3d%3d";
wp_enqueue_script( $src, $src, array( "jquery"), null );

produces

<script type='text/javascript' src='http://www.example.com/widget.js?token=something%3D%3D&#038;ver=token=something%3d%3d'></script> 

There are two problems here

  1. the ver= parameter is still being created
  2. The %3d part of the token was unexpectedly capitalized to %3D in the first occurrence of token=

The problem is in do_item() ( wp-includes/class.wp-scripts.php )

The conversion of %3d to %3D is performed by add_query_arg('ver', $ver, $src);
Up to this point $src was the original (wanted) string.
Had add_query_arg() not been invoked the $src would have been as required.

My questions are these?
Is this a bug in the code or a bug in the documentation?
If not a bug in the code then HOW can I call wp_enqueue_script() so that my required $src IS respected?
What if I really did require '%3d' to be passed as '%3d'?

Attachments (1)

24834.patch (476 bytes) - added by bobbingwide 11 years ago.
replace !empty( $ver) by null !== $this->registered[$handle]->ver

Download all attachments as: .zip

Change History (18)

@bobbingwide
11 years ago

replace !empty( $ver) by null !== $this->registered[$handle]->ver

#1 follow-up: @johnbillion
11 years ago

Off the top of my head, I think the Codex is incorrect. You should pass boolean false as the $ver parameter to prevent the 'ver' query var being appended to the script URL.

#2 in reply to: ↑ 1 @bobbingwide
11 years ago

Replying to johnbillion:

Off the top of my head, I think the Codex is incorrect. You should pass boolean false as the $ver parameter to prevent the 'ver' query var being appended to the script URL.

false (the default) is a request to use the WordPress version.

#3 @bobbingwide
11 years ago

my patch ignores all the messing about with the $ver field and retests the value of $this->registered[$handle]->ver
If the value is not null then the required version string is added.

#4 @pauldewouters
11 years ago

I coudn't reproduce this in version 3.7-alpha-24954, using null works as expected, even with a query arg

Last edited 11 years ago by pauldewouters (previous) (diff)

#5 @pauldewouters
11 years ago

  • Cc pauldewouters@… added

#6 @helen
11 years ago

  • Keywords close added

Weird things happen if you have a query string in the handle. You can chase the whys like I did for a little while, or just give it a proper handle name, like example-widget or something. :)

#7 follow-ups: @bobbingwide
11 years ago

@Pauldewouters. Please try again because I can reproduce the problem very easily using the oik base plugin (v2.1-alpha.0802) AND the following shortcode

[bw_jq src="http://www.example.com/widget.js?token=somethingcomplicated%3d%3d"]

@helen There should be no need to give it a proper handle name if you apply the fix.

I could be invoking the JavaScript multiple times with different parameters.
So, in order to make it unique, the handle name has to contain the query string.

I know weird things happen. My patch is a pragmatic solution to the problem.



#8 in reply to: ↑ 7 ; follow-up: @pauldewouters
11 years ago

Replying to bobbingwide:

@Pauldewouters. Please try again because I can reproduce the problem very easily using the oik base plugin (v2.1-alpha.0802) AND the following shortcode

[bw_jq src="http://www.example.com/widget.js?token=somethingcomplicated%3d%3d"]

right, I didn't notice the handle was dynamic too

#9 in reply to: ↑ 8 @bobbingwide
11 years ago

Replying to pauldewouters:

Some more information... I'll try to answer my own questions.

  1. In my experience when the value for the $handle parameter contains a query string ( such as when it matches the $src parameter, as in my example) then the API doesn't work as expected... the value of null for $ver is ignored.

The patch I provided works for 3.5.2, 3.6 and 3.7-alpha-24954. But it may cause the special case logic to fail. The special case logic is documented in the codex as:

As a special case, if the string contains a '?' character, the preceding part of the string refers to the registered handle, and the succeeding part is appended to the URL as a query string.

  1. As a workaround I have changed my code to use the PHP uniqid() function to provide the $handle
    $src = "http://www.example.com/widget.js?token=something%3d%3d";
    wp_enqueue_script( uniqid(), $src, array( "jquery"), null );
    

Perhaps THIS should be documented in the codex? I wonder how many people are making use of the special case logic.

#10 follow-up: @nacin
11 years ago

Why are you using a uniqid() for $handle? Use a real string like 'my-example-widget' or something.

I doubt many people are using this "special" logic — and it likely just predates extra arguments like $ver and such. I'd have to look. But we're not going to remove it. Proper handle names are required. This is a case of doing it wrong.

#11 @nacin
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

#12 @helen
11 years ago

I updated the Codex.

#13 in reply to: ↑ 10 @bobbingwide
11 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to nacin:

Why are you using a uniqid() for $handle?

See my response to helen

This is a case of doing it wrong.

I have to "do it wrong" in order to get it to work.

Using uniqid() ensures that each instance will get enqueued.

Would you prefer it if I coded it like this?

// I'm not telling you how I set $src_uniq but uniqid() might be one method!
// or this may be another $src_uniq = str_replace( array( "?" ), "-", $src );
wp_register_script( $src_uniq, $src, array( "jquery"), null );
wp_enqueue_script( $src_uniq  ); 

Using this technique allows me to specify the query parameters in $src AND pass a null value for $ver.

If I try to specify the query parameters in the $handle and use $ver=null then it won't work

wp_enqueue_script( "fladdle2?parm=ploing%2d%2D", "http://qw/bw/jq.js", array( "jquery"), null ); 


gives

<script type='text/javascript' src='http://qw/bw/jq.js?ver=parm=ploing%2d%2D'></script>

This is the restriction that Helen has documented.

For the record, results you may expect to get with non-null values for $ver are:

wp_enqueue_script( "fladdle3?parm=ploing%3d%3D", "http://qw/bw/jq.js", array( "jquery"), false );

gives

<script type='text/javascript' src='http://qw/bw/jq.js?ver=3.7-alpha-24954&#038;parm=ploing%3d%3D'></script>

which seems OK. Note: No conversion of %3d to %3D

AND

wp_enqueue_script( "fladdle4?parm=ploing%4d%4D", "http://qw/bw/jq.js?farm=floing%4d%4D", array( "jquery") ); 

gives

<script type='text/javascript' src='http://qw/bw/jq.js?farm=floingMM&#038;ver=3.7-alpha-24954&#038;parm=ploing%4d%4D'></script>

where the query parms on the $src are parsed and re-created by add_query_arg(),
which may produce different results from expected.

#14 follow-up: @helen
11 years ago

What exactly is it you are trying to accomplish here? The handle can be anything, so name it my-example-widget-2.0 and my-example-widget-2.4 or something. It just has special behavior when a query string is involved. If you have to generate it programmatically for some reason, replacing the ? sounds good, or try a sanitize function like sanitize_key().

Your patch would change current behavior. Note that $src can be altered before its usage in that conditional.

#15 in reply to: ↑ 7 @azaozz
11 years ago

Replying to bobbingwide:

...
I could be invoking the JavaScript multiple times with different parameters.

You're actually not invoking the JavaScript, you're loading it again and again. Passing multiple arguments to a script can be done in a more effective way from another script or number of scripts.

So, in order to make it unique, the handle name has to contain the query string.
...
In my experience when the value for the $handle parameter contains a query string ( such as when it matches the $src parameter, as in my example) then the API doesn't work as expected... the value of null for $ver is ignored.

The handle is the internal name of the script. It supports URL query type syntax as a not recommended back-compat only. Anything passed after the ? in a script handle is treated (and outputted) as cache busting URL query.

Btw, perhaps it's time to remove that back-compat bit from script handles. It was a back-compat even 5 years ago. Think it's a left over from before WP_Scripts supported explicit cache-busting arg.

Trying to guess what you want to do: load the same script multiple times with different query strings (quite ineffective as the script will be fully loaded each time, on top it will run multiple times that may cause unexpected results). The needed query string can be in $src and a static in your shortcode handling function can be used to make the handle unique:

function _my_shortcode_handler() {
  static $number = 1;
  
  wp_enqueue_script( 'my-script-' . $number, $src, array("jquery"), ... );
  $number++;
}

Suggesting closing as invalid.

Last edited 11 years ago by azaozz (previous) (diff)

#16 in reply to: ↑ 14 @bobbingwide
11 years ago

Replying to helen:

What exactly is it you are trying to accomplish here?

I'm trying to pass a query string but not have ver= set
This is to cater for a restriction with some external JavaScript that I have no control over.

There is a possibility that the same URL could be invoked multiple times with different query strings where the server delivers different JavaScript source depending on the value of the parameters.

Or try a function like sanitize_key()

Yes, that's tidier.

Your patch would change current behavior.

Well that was the plan! The current behaviour doesn't support $ver=null.
But I agree that my patch doesn't cater for the query parms in the handle.

Replying to azaozz:
IMHO, the defect is valid, but there is a work around.
So perhaps the defect should be closed as wontfix, for all the reasons given above.

Another scenario is when the same shortcode gets expanded in multiple places on the page.
If the handles are identical then the script should only be loaded once.
In this scenario the uniqid() and $number++ solutions won't work but the sanitize_key() solution does.

#17 @helen
11 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

So, there are solutions here, then. Having a query string in the handle is a known special case, and if you're not trying to take advantage of that very special, very old case, you should alter the handle and use a properly named one.

Note: See TracTickets for help on using tickets.