Make WordPress Core

Opened 11 years ago

Closed 6 years ago

Last modified 10 months ago

#26886 closed defect (bug) (fixed)

wp_enqueue_script doesn't load all scripts

Reported by: alfredocubitos's profile alfredocubitos Owned by: azaozz's profile azaozz
Milestone: 5.3 Priority: normal
Severity: normal Version: 3.8
Component: Script Loader Keywords:
Focuses: Cc:

Description

trying to load scripts with wp_enqueue_script produces the following output:

wp-admin/load-scripts.php?c=1&load%5B%5D=hoverIntent,common,admin-bar,svg-painter,heartbeat,jquery-ui-datep&load%5B%5D=icker,jquery-ui-resizable,..,jquery-ui-to&load%5B%5D=oltip&ver=3.8'>

sometimes where this characters inserted:

&load%5B%5D=

e.g.

jquery-ui-to&load%5B%5D=oltip
instead jquery-ui-tooltip

Please see also this support thread

link:http://wordpress.org/support/topic/widgets-not-drag-or-drop-after-update-to-38?replies=3#post-5128942

Attachments (2)

26886.diff (2.7 KB) - added by dd32 10 years ago.
Screen Shot 2015-02-24 at 11.22.25 am.png (45.0 KB) - added by dd32 10 years ago.

Download all attachments as: .zip

Change History (43)

#1 @jeremyfelt
11 years ago

  • Component changed from General to Script Loader
  • Keywords reporter-feedback added

Hi @alfredocubitos, thanks for the report.

Can you provide steps to reproduce this issue in WordPress 3.8? It sounded like you had not been able to reproduce this on a local installation yet.

#2 @alfredocubitos
11 years ago

first I made an upgrade. After this upgrade everything was ok.

After a clean new installation the problem occurred also in my local installation.

I looked around in the wp codes and I found the problem characters in

function _print_scripts()
.
:
      $concat = 'load%5B%5D=' . implode( '&load%5B%5D=', $concat );

in https://core.trac.wordpress.org/browser/tags/3.8.1/src/wp-includes/script-loader.php at line 786

Please also have a look at this support thread.

http://wordpress.org/support/topic/widgets-not-drag-or-drop-after-update-to-38?replies=3#post-5128942

Marci73 shows a solution that works.

I hope this may help finding the problem for this bug.

#3 @nacin
11 years ago

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

Hi alfredocubitos, These characters are valid. I think this is probably a bug in your theme or plugin — whichever is overriding scripts in the admin or otherwise causing a syntax error or conflict.

#4 follow-up: @kraterdesign
10 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I just ran into this bug and also found why and when it happens.

In latest WP, check out function _print_scripts() in script-loader.php (line 848-849 in current v4.0)

                $concat = str_split( $concat, 128 );
                $concat = 'load%5B%5D=' . implode( '&load%5B%5D=', $concat );

That first line splits the string $concat into an array of strings of max. length 128. So when you have a string of comma-separated script names that exceeds 128 characters, the script name that crosses the 128th character boundary inside the string, will be cut in two and thus not be loaded.

As you can see this only happens when there are a lot of admin scripts loaded. Ie. when concatenated into a comma-separated they make up a string with a total length larger than 128 chars.

When I ran into this, the concatenated end result reads:

/wp-admin/load-scripts.php?c=0&load%5B%5D=jquery-core,jquery-migrate,underscore,utils,jquery-ui-core,jquery-ui-widget,jquery-ui-mouse,jquery-ui-sortable,jquery-ui-datepic&load%5B%5D=ker,json2,jquery-ui-draggable&ver=4.0

... in this case effectively not loading jquery-ui-datepicker.

I'm guessing this was done to make sure the parameter value length never exceeds 128 chars. Here's a different way to do the same and not break script names.

                $concat = explode( $concat, ',' );
                $concats_128char = array();
                $concat_temp = '';
                foreach ( $concat as $c ) {
                        if ( strlen( $concat_temp . ',' . $c ) > 128 ) {
                                $concats_128char[] = $concat_temp;
                                $concat_temp = '';
                        }
                        $concat_temp .= ',' . $c;
                        $concat_temp = trim( $concat_temp, ',' );
                }
                $concats_128char[] = $concat_temp;
                $concat = 'load%5B%5D=' . implode( '&load%5B%5D=', $concats_128char );

#5 @dd32
10 years ago

... in this case effectively not loading jquery-ui-datepicker.

This shouldn't be the case, as the string is merged together:

wp-admin/load-scripts.php:
$load = $_GET['load'];
if ( is_array( $load ) )
	$load = implode( '', $load );

?load[]=abc,d&load[]=ef gets combined to $load="abc,def" before being split by commas and processed.

It's worth noting though, that loading a url containing load%5B%5D into your browser URL bar will result in an invalid URL, you need to replace load%5B%5D with load[] to test that way, it's the proper format for use In a <script> tag though.

#6 @kraterdesign
10 years ago

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

Hi dd32, you sir, are of course correct and I stand humbly corrected. I had not looked farther down the line.

I looked into it some more and it turns out I was having a plugin conflict between Option-Tree (in particular the scripts for custom meta boxes) and BirchPress Business that seemed solved by doing

define('CONCATENATE_SCRIPTS', false);

so I ended up blaming the wrong thing. Sorry. I solved it properly by preventing the loading of OT scripts on BP post type pages.

Why the lack of concatenation in this conflict case prevented any JS errors I was otherwise seeing, I have not investigated further.

Version 0, edited 10 years ago by kraterdesign (next)

#7 in reply to: ↑ 4 @alturic
10 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

Replying to kraterdesign:

I just ran into this bug and also found why and when it happens.

In latest WP, check out function _print_scripts() in script-loader.php (line 848-849 in current v4.0)

                $concat = str_split( $concat, 128 );
                $concat = 'load%5B%5D=' . implode( '&load%5B%5D=', $concat );

That first line splits the string $concat into an array of strings of max. length 128. So when you have a string of comma-separated script names that exceeds 128 characters, the script name that crosses the 128th character boundary inside the string, will be cut in two and thus not be loaded.

As you can see this only happens when there are a lot of admin scripts loaded. Ie. when concatenated into a comma-separated they make up a string with a total length larger than 128 chars.

When I ran into this, the concatenated end result reads:

/wp-admin/load-scripts.php?c=0&load%5B%5D=jquery-core,jquery-migrate,underscore,utils,jquery-ui-core,jquery-ui-widget,jquery-ui-mouse,jquery-ui-sortable,jquery-ui-datepic&load%5B%5D=ker,json2,jquery-ui-draggable&ver=4.0

... in this case effectively not loading jquery-ui-datepicker.

I'm guessing this was done to make sure the parameter value length never exceeds 128 chars. Here's a different way to do the same and not break script names.

                $concat = explode( $concat, ',' );
                $concats_128char = array();
                $concat_temp = '';
                foreach ( $concat as $c ) {
                        if ( strlen( $concat_temp . ',' . $c ) > 128 ) {
                                $concats_128char[] = $concat_temp;
                                $concat_temp = '';
                        }
                        $concat_temp .= ',' . $c;
                        $concat_temp = trim( $concat_temp, ',' );
                }
                $concats_128char[] = $concat_temp;
                $concat = 'load%5B%5D=' . implode( '&load%5B%5D=', $concats_128char );

I know this is super out-dated, but in my regard on WP 4.1 this is still an issue. Sure it's my theme calling a ton of scripts but it's fixed by changing $concat = str_split( $concat, 128 ); to something higher? I changed it to $concat = str_split( $concat, 3000 ); and everything SEEMS to work fine, I don't have the problem anymore or anything but a.) it's editing a core file (blah) and b.) I'm unsure what all can happen by simply doing that.

#8 @SergeyBiryukov
10 years ago

  • Milestone set to Awaiting Review

#9 @kraterdesign
10 years ago

@alturic, see (comment 5), when the scripts are loaded the split query strings are concatenated again. So your problem is, like mine was, unrelated to the splitting, which itself couldn't be a problem.

#10 follow-ups: @alturic
10 years ago

So, WP itself conc's it at 128 chars. It's cutting script names off in the middle of the script name, and you're chalking it up to FireBug or some non-WordPress issue? WP >IS< the one conc'ing it at 128 chars it's nothing BUT WP's problem.

As for it being a plugin issue. I mean, unless a WP plugin can alter WP Core functionality (I have no idea? It'd be kinda crazy to allow a plugin to do so though) the only thing you can blame on the plugin is calling scripts.

Curious what you're thoughts are on at though, simply telling WP to by-pass conc'ing isn't a fix nor should it be expected. Like I said, I simply changed it from 128 to 3000 (arbitrary number) and all worked well I just don't know what side-effects will happen from that.

#11 in reply to: ↑ 10 @kraterdesign
10 years ago

Replying to alturic:

So, WP itself conc's it at 128 chars. It's cutting script names off in the middle of the script name, and you're chalking it up to FireBug or some non-WordPress issue? WP >IS< the one conc'ing it at 128 chars it's nothing BUT WP's problem.

As for it being a plugin issue. I mean, unless a WP plugin can alter WP Core functionality (I have no idea? It'd be kinda crazy to allow a plugin to do so though) the only thing you can blame on the plugin is calling scripts.

Curious what you're thoughts are on at though, simply telling WP to by-pass conc'ing isn't a fix nor should it be expected. Like I said, I simply changed it from 128 to 3000 (arbitrary number) and all worked well I just don't know what side-effects will happen from that.

Yes, it's split at 128 chars to form the url that's enqueued and output into HTML. But when "/wp-admin/load-scripts.php?....etc" with the split query strings is called by the browser, it first reassembles the split string into one long string and only then loads the scripts from it. I didn't realize this myself until it was pointed out to me by dd32. So it's never an issue and the 128 limit is just for backwards compatibility reasons with regards to the HTTP protocol AFAIK.

The way this is done, as far as the successfully loading of scripts goes, it wouldn't matter if it was cut off at 12, 37 or 128. Since the query strings are merged before the script concatenator does anything with the script names, it would always work.

Last edited 10 years ago by kraterdesign (previous) (diff)

#12 @georgestephanis
10 years ago

Just to clarify, the reason it's chunked is to account for some server restrictions, such as suhosin.get.max_value_length which defaults to 512.

See: r23074 and #22757

#13 @SergeyBiryukov
10 years ago

#31371 was marked as a duplicate.

#14 in reply to: ↑ 10 ; follow-up: @SergeyBiryukov
10 years ago

Replying to alturic:

So, WP itself conc's it at 128 chars. It's cutting script names off in the middle of the script name, and you're chalking it up to FireBug or some non-WordPress issue? WP >IS< the one conc'ing it at 128 chars it's nothing BUT WP's problem.

As noted in comment:5:ticket:26886 and comment:11:ticket:26886, the string is merged back together before being split by commas and processed, so the 128 character limit shouldn't make a difference.

Looks like the issue is specific to your environment. Does it still happen with all plugins disabled and a default theme (Twenty Fifteen or Twenty Fourteen) activated?

As for it being a plugin issue. I mean, unless a WP plugin can alter WP Core functionality (I have no idea? It'd be kinda crazy to allow a plugin to do so though) the only thing you can blame on the plugin is calling scripts.

Every plugin alters WordPress core functionality in some way, sometimes breaking it.

#15 in reply to: ↑ 14 @alturic
10 years ago

Replying to SergeyBiryukov:

Let me ask you this, is it possible if breaks when more than two load[] Query String lines occur? Of course it doesn't break when using a default theme and no plugins, there's very few scripts being loaded. I've only noticed it breaking when there's 3+ load[] Query Strings.

Outside of that, when I say a plugin breaking WP Core functionality, I meant in the sense that a theme/plugin being able to actually alter the load[] function to where it'd be causing it to break the concat/rebuild function.

Lastly, is there anything to worry about simply changing that from 128 to something higher? 300? 3000? etc...

#16 follow-up: @dd32
10 years ago

Let me ask you this, is it possible if breaks when more than two load[] Query String lines occur?

It's possible that your server configuration is setup to break requests like that, yes. It could be some kind of security plugin kicking in or something.

Lastly, is there anything to worry about simply changing that from 128 to something higher? 300? 3000? etc...

Yes, Servers which limit the maximum query var value length, using some kind of security plugin..

/wp-admin/load-scripts.php?c=0&load%5B%5D=jquery-core,...

Also worth noting, that if you want to take that URL from the page source (or network inspector in some browsers) and request that using the browser (by putting it into the address bar) you need to properly decode load%5B%5D= to load[]= otherwise it'll seemingly not load anything (or load something wrong). In other words - load%5B%5D= is valid (and correct) in the page source, but not when placed into the address bar.

#17 in reply to: ↑ 16 @alturic
10 years ago

Replying to dd32:

Let me ask you this, is it possible if breaks when more than two load[] Query String lines occur?

It's possible that your server configuration is setup to break requests like that, yes. It could be some kind of security plugin kicking in or something.

So if it's literally WP that's "breaking" (note, I use that loosely) and I know the exact code that's causing it, down to the char length, how can it be the "server" outside of saying "because other's don't have the issue"?

Lastly, is there anything to worry about simply changing that from 128 to something higher? 300? 3000? etc...

Yes, Servers which limit the maximum query var value length, using some kind of security plugin..

So basically if I know (when changing that to a crazy value), it then works correctly, nothing with WP at least will break putting that higher so WP doesn't concat the string?

/wp-admin/load-scripts.php?c=0&load%5B%5D=jquery-core,...

Also worth noting, that if you want to take that URL from the page source (or network inspector in some browsers) and request that using the browser (by putting it into the address bar) you need to properly decode load%5B%5D= to load[]= otherwise it'll seemingly not load anything (or load something wrong). In other words - load%5B%5D= is valid (and correct) in the page source, but not when placed into the address bar.

Yea, that's been my point, it loads perfectly fine if I a.) convert it to load[] in the url and b.) put those load[]'s not in the middle of script-names. :P

@dd32
10 years ago

#18 follow-up: @dd32
10 years ago

So if it's literally WP that's "breaking" (note, I use that loosely) and I know the exact code that's causing it, down to the char length, how can it be the "server" outside of saying "because other's don't have the issue"?

Because the code that joins it back together uses whatever is passed to WordPress from the server. There's literally nothing in WordPress that can cause what you're claiming - Unless you have a plugin breaking JS, a server screwing with the request, or an install that's been modified (either files missing, corrupt files, or something else).

Yea, that's been my point, it loads perfectly fine if I a.) convert it to load[] in the url and b.) put those load[]'s not in the middle of script-names. :P

Then it kind of sounds like it's working as expected, based on a) only being needed for testing, and b) not understanding how it works.

26886.diff is a debug change you can make, it causes no JS to load, but will display an alert dialogue when you visit a dashboard page with the scripts that it's attempting to load, The image shows what you'd expect to see, Load= shows a proper string after processing, no broken handle names. $_GET['load'] shows a break every 40 characters ( in inline-edit-post and wp-auth-check) which is what the browser passes (correctly).

#19 in reply to: ↑ 18 @alturic
10 years ago

Replying to dd32:

Yea, that's been my point, it loads perfectly fine if I a.) convert it to load[] in the url and b.) put those load[]'s not in the middle of script-names. :P

Then it kind of sounds like it's working as expected, based on a) only being needed for testing, and b) not understanding how it works.

Well, and I don't mean this rudely, it works perfectly fine with I manually putting the load[]'s in the url NOT in the middle of script-names, which is what's happening. Perhaps I didn't make that part clear? The actual function is "working", it's just literally inserting the additional load[]'s in the middle of script names...

So for example if jquery-ui-tabs was the script name and the "j" in jqeury-ui-tabs was char 120, the function would end up making the query string parameter:

URL:

load%5B%5D=scriptnames,up,to128,chars,jqeury-uiload%5B%5D=-tabs,more,scriptnames,pretendthisischar125,thload%5B%5D=iswasbroken,aswell

Query String Parameter:

load[]=scriptnames,up,to120,chars,jqeury-ui
load[]=-tabs,more,scriptnames,pretendthisischar125,th
load[]=iswasbroken,aswell

Note the two broken script names in the above.

Now when things are working correctly it would look like this I'm guessing:

load%5B%5D=scriptnames,up,to128,charsload%5B%5D=jqeury-ui-tabs,more,scriptnames,pretendthisischar125load%5B%5D=thisnowaworkingscriptname,aswell

Query String Parameter:

load[]=scriptnames,up,to120,chars
load[]=jquery-ui-tabs,more,scriptnames,pretendthisischar125
load[]=thisisnowaworkingscript,aswell

Note in the above two it's knowing "hey, we want to goto 128 chars, but we don't want to insert additional load[]'s in the middle of script names.

And that's what I'm saying, the WP function is the one who's concating the length at 128, so how would nginx or php be causing it to not be smart (as in "finishing" the actual concat function) about the script name as to not insert a load[] in the middle of names...

As for the stack being the true culprit (and I'm not saying it's not, just questioning how (seemingly) a portion of the function runs but the second-half of not inserting the load[]'s isn't functioning properly) well, frankly, I'd be at a loss as that seems to literally be the only thing having issues. :-/

Last edited 10 years ago by alturic (previous) (diff)

#20 @dd32
10 years ago

Perhaps I'm not being clear then, when I say that it doesn't matter if it's cut at 5 characters, or 5,000 characters, it still gets used the same inside WordPress.

load[]=scriptnames,up,to120,chars,jqeury-ui
load[]=-tabs,more,scriptnames,pretendthisischar125,th
load[]=iswasbroken,aswell

gets converted to

$load="scriptnames,up,to120,chars,jqeury-ui-tabs,more,scriptnames,pretendthisischar125,thiswasbroken,aswell"

by using

$load = implode( '', $_GET['load'] );

It doesn't matter where it's cut, at the end of a handle name, or in the middle of one, it just concats them all together, and then processes it.
Look at the value of $load within load-scripts.php, and what that is seeing in $_GET, and come back with that, it'll show that the cutting isn't the issue you're seeing.

Last edited 10 years ago by dd32 (previous) (diff)

#21 @alturic
10 years ago

http://i.imgur.com/os5KFTf.png

So, and I already knew it was "getting" the script/script names properly just inserting the load[]'s literally at 128 chars, but I'm guessing this is what you mean by WordPress isn't the one causing the issue?

Something is just causing load[] to literally get inserted at 128 chars and not "realize" the it's inserting it in the middle of script names causing those scripts not to load and because the function is set to 128 chars I figured it was WordPress.

Oh, and this is no plugins, 2015 theme.

EDIT: After reading your last statement, load[] and/or load%5B%5D= getting inserted in the middle of script-names doesn't actually matter at all (it still loads them?), but see below which is what got me down this rabbit-hole. That is my true "problem", which I'm unsure if it's related to this actual problem pending the answer to the question above.

Last edited 10 years ago by alturic (previous) (diff)

#22 @alturic
10 years ago

So, the ##php guys can't figure out any sort of config flag that would cause it, and again, with no plugins and the 2015 theme this is what Chrome's debugger shows, which is what got me started on the problem to begin with:

http://i.imgur.com/9UQHUn9.png

EDIT: Ok, let me ask this @dd32: On a "working" WP 4.1 install, should any JS errors throw in a browser console?

Uncaught TypeError: Cannot read property 'each' of undefined - no plugin/2015 theme "edit page" page? Line 1 of

!function(a,b){if("function"==typeof define&&define.amd)define(["underscore","jquery","exports"],function(c,d,e){...

Uncaught TypeError: Cannot read property 'model' of undefined - no plugin/2015 theme "add new page" page? Perhaps due to it being a new page and no media being attached?

c.queue=new wp.media.model.Attachments([],{query:!1})

is the property it looks like.

Last edited 10 years ago by alturic (previous) (diff)

#23 @alturic
10 years ago

Ok, so final update before a reply from me, I promise.

I went through and made an image to show side-by-side difference between a "working" install, and this (again, default 4.1.1, no plugins) install with the only difference being nginx instead of apache in the stack. The nginx guys say they have no idea what type of config flag could be causing this, the php guys say the same soooo.

http://i.imgur.com/j9CifRl.png

#1 is the "working" install. #2 is the "bad" install. Now, I'm over the actual load[] statement stuff, but I didn't want to open a new ticket "just because", so we'll see if it is somehow possibly related to the above comments... but from the get-go, the complete default install is loading more scripts than a customized install. I don't know WP Core scripts or anything but wp-ajax-response, quicktags, jquery-jquery, admin-comments all sound like fairly needed things, hence the hesitation.

#24 @dd32
10 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from reopened to closed

As far as I can tell, nothing you've posted indicates it's a bug in Core.

Test on a clean install, no plugins, no custom themes, fresh download from WordPress.org, if you're still having issues, you can contact me off-trac at dion @ wordpress.org with some site details and I'll personally help you out.

#25 @k4v
9 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I just ran into the exact same problem.

This happened when I moved an older WP instance to a new Server with nginx, PHP5.6, php-fpm.

For me the solution was to hack script-loader.php and set the str_split parameter to 1024

Last edited 9 years ago by k4v (previous) (diff)

#26 @dd32
9 years ago

@k4v Can you offer any debugging information to explain why it broke for you?

For reference, we can't increase the limit significantly (as you've done) as it breaks when used in conjunction with some security limit software such as suhosin.

Some things that would help include, the URLs generated, and what the content of it was. The site URL could also be beneficial to debugging (Feel free to email me that offline - dion at wordpress.org)

#27 @SergeyBiryukov
9 years ago

  • Milestone set to Awaiting Review

#28 @ocean90
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from reopened to closed

#29 @SergeyBiryukov
8 years ago

#39721 was marked as a duplicate.

#30 @roblesterjr
8 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

We found this issue while troubleshooting why the admin panel did not work while being passed through the Fastly service. For some reason, the issue doesn't cause any issues when used normally, but when passed through fastly, it causes the scripts not to be brought in.
This code:

$concat = str_split( $concat, 128 );
		$concat = 'load%5B%5D=' . implode( '&load%5B%5D=', $concat );

Found at line 1146 in script-loader.php looks to be unnecessary. When I comment it out, and just prepend load[]= to the $concat value, the admin scripts load fine without the extra injection.
Here is an example of what i'm finding in the query string:

jquery-ui-widget,jq&load%5B%5D=uery-ui-mouse,jquery-ui-sortable

We'd love it if you would address this. Thank you.

#31 @SergeyBiryukov
8 years ago

  • Milestone set to Awaiting Review

#32 @roblesterjr
8 years ago

You can observe the malformed script query string by looking at the sources in the inspector while on the media page in the admin screen. This is where we are finding the problem.

#33 follow-up: @emanueleivaldi
8 years ago

Hi all,

I don't think it's a good idea to arbitrary split the $concat without worrying about its actual content.

This can cause unexpected issues because it inherently relies on the fact that the order of the query parameters will be maintained.

When this doesn't happen (e.g. the reverse proxy in front of your application reorders the query parameters) wordpress won't be able to reconstruct the original string

Imagine you have the string "this,is,just,an,example"

if you arbitrary split after n characters (10 for this example) you will end up with

?load[]=this,is,ju&load[]=st,an,exam&load[]=ple

if you keep the order of the query parameters you can reconstruct the original string but if you don't you can end up having something like this:

?load[]=stan,exam&load[]=ple&load[]=this,is,ju

from these query params you only reconstruct a few words

"this", "example" and "is" and you are going to lose "just" and "an".

If the logic to maintain every query parameter within a reasonable size would pay attention to the actual content and avoids truncating words this problem will disappear.

Last edited 8 years ago by emanueleivaldi (previous) (diff)

#34 @nickbreen
7 years ago

I've run into this issue when the front-end webserver or proxy does not support multiple same-name query parameters. In my particular case AWS API Gateway.

A slight difference is that the location of the break is irrelevant; that there is a break at all (rather than it generates multiple load[] parameters) is the problem.

Using CONCATENATE_SCRIPTS to work around it is good enough though.

Last edited 7 years ago by nickbreen (previous) (diff)

#36 @azaozz
6 years ago

Anybody that had problems with this, could you test the patch on #45346 (https://core.trac.wordpress.org/attachment/ticket/45346/45346.diff). It fixes the (for now) identified problems:

  • The server sorting the load array which messes up the string.
  • There are no "multiple same-name query parameters". (Although that's an absurd restriction. Arrays in the query string always use multiple same-name params. This is probably affecting other query strings in WP.)

#37 in reply to: ↑ 33 ; follow-up: @azaozz
6 years ago

Replying to emanueleivaldi:

if you keep the order of the query parameters you can reconstruct the original string but if you don't...

Why would you "mess up" the order of an array and expect it to work properly. It is not the same array any more... Even if we had one script per array element, the script loading order would change and it would still fail.

Anyway, the above patch should fix this.

#38 in reply to: ↑ 37 ; follow-up: @emanueleivaldi
6 years ago

Replying to azaozz:

Replying to emanueleivaldi:

if you keep the order of the query parameters you can reconstruct the original string but if you don't...

Why would you "mess up" the order of an array and expect it to work properly. It is not the same array any more... Even if we had one script per array element, the script loading order would change and it would still fail.

Anyway, the above patch should fix this.

Hi, we sorted the query parameters at the cdn level in order to increase the hit ratio on cached pages.

url?a=1&b=1 would not generate the same cache key of url?b=1&a=1

Thanks for fixing it

Last edited 6 years ago by emanueleivaldi (previous) (diff)

#39 in reply to: ↑ 38 @azaozz
6 years ago

Replying to emanueleivaldi:

we sorted the query parameters at the cdn level in order to increase the hit ratio on cached pages.

Yeah, that's a good optimization (although not sure how often it gets used). However this should not happen when the query parameters form an array, like in this case. Perhaps you should look at the param "names" and if more than one contain [ and ] just leave the query string as-is :)

In PHP this should be as simple as:

if ( substr_count( $query_string, ']=' ) < 2 ) {
    // sort
}

Assuming the patch from https://core.trac.wordpress.org/attachment/ticket/45346/45346.diff works well. Going to commit it and close this ticket with it too. Please feel free to reopen with more info and examples if it doesn't fix the bug(s) reported here.

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

#40 @azaozz
6 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from reopened to closed

In 45456:

Script loader: prevent sorting of the load array in the query string when passing the script handles to load-scripts.php and load-styles.php.

Fixes #45346 #26886.

#41 @desrosj
6 years ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 5.3
Note: See TracTickets for help on using tickets.