WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#27846 closed defect (bug) (invalid)

Removing query string from style link causes TinyMCE link button to display at bottom of page

Reported by: Autumn Lansing Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.9
Component: TinyMCE Keywords:
Focuses: Cc:

Description

I found an interesting bug with the new 3.9 release. There's a standard bit of code around the internet which lets you remove the query string from style and script links in the head so that they can be cached. The code is this:

function remove_wp_ver_css_js( $src ) {
  if ( strpos( $src, 'ver=' ) )
  $src = remove_query_arg( 'ver', $src );
  return $src;
}

Using that code now breaks the pop-up window for TinyMCE's "link" button. The contents of that window instead display at the bottom of the page. This only happens when calling it to remove the query strings on styles, in this manner:

add_filter( 'style_loader_src', 'remove_cssjs_ver', 10, 2 );

Using it with "script_loader_src" instead of "style_loader_src" does not cause this bug.

I know this code isn't part of core, but I'm assuming that the bug is an issue with TinyMCE.

Change History (7)

#1 @Autumn Lansing
6 years ago

I should add: this only happens in Chrome for me, not Firefox. I'm using Chrome 34.

Last edited 6 years ago by Autumn Lansing (previous) (diff)

#2 @nacin
6 years ago

Could that file still be cached? It changed extensively in 3.9.

For what it's worth, the reason why ver= exists is to bust caches with each new version, to account for file updates. The files themselves are still cacheable, of course. I'm not sure why someone would want to use this snippet for caching purposes.

#3 @Autumn Lansing
6 years ago

No, caching is not being used on my development machine. I've also tested it on three different installs with the same result. It worked up to 3.8.3, but not in 3.9.

Some proxy caching services won't cache files with query strings. Google Page Speed also dislikes query strings, and it's believed to have an effect on rankings. Some also prefer to place their own query strings with the file's actual version other than the WP version.

#4 @azaozz
6 years ago

Don't think we can avoid using query strings for cache management. They (should) trigger cache updates on caching proxies and in the browsers. The other way of "on demand" cache busting would need access to the web server configuration, which is not possible from PHP in all cases.

If a plugin removes the JS and CSS query strings, I'm hoping that it also sets the proper headers at the web server level.

#5 @SergeyBiryukov
6 years ago

Using that code now breaks the pop-up window for TinyMCE's "link" button.

Could not reproduce. The "Insert/edit link" modal works as expected for me with this snippet in current trunk.

#6 @Autumn Lansing
6 years ago

I did a bit more playing around with this. That snippet was definitely the cause. I unplugged most everything in the themes I was using to test, and the bug was occurring only when that line of code was enabled. However, it seemed to have fixed itself somehow, in a manner which makes this bug even more odd. In my tests, I was simply trying to open the pop-up window, and that would cause the bug, but when I actually created a link through the pop-up on one test page, the bug disappeared after that, and not only did it disappear for that theme, it disappeared for the other unrelated test themes as well.

Not sure what caused it, but I suspect it was a browser bug of some type, as it was only happening in Chrome, which would also explain why it fixed itself in unrelated themes. It's probably not something to worry about. Feel free to close this.

@azaozz: I think what WordPress does by default is probably best for the average user. If anyone is informed enough to know they need something special, they're probably capable of implementing it themselves. Though I do know that many people also consider it a security risk to have their WP version number so easily readable, so that's another reason someone might remove the query string.

#7 @iseulde
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.