Opened 10 years ago
Closed 10 years ago
#29383 closed defect (bug) (fixed)
WPPlaylistView doesn't find the right script.
Reported by: | programmin | Owned by: | azaozz |
---|---|---|---|
Milestone: | 4.0 | Priority: | normal |
Severity: | normal | Version: | 4.0 |
Component: | Media | Keywords: | has-patch commit dev-reviewed |
Focuses: | Cc: |
Description
WPPlaylistView is looking for any script, then getting the contents, when it's initializing the playlist. On my test setup this is grabbing a different script, a google-analytics ga.js.
Please change wp-playlist.js to look for
this.$('script[type="application/json"]').html()
instead of
this.$('script').html()
in initialize function.
Attachments (2)
Change History (19)
#2
follow-up:
↓ 3
@
10 years ago
Honestly I don't know where the theme was shoving more script tags into it right now. Regardless I think it should be possible to add script tags, and not have the playlist blindly grab the first script tag's contents.
It would also be much more readable to grab it like this, as getting the html() contents of a script tag is rarely done, and this would clarify that it is getting script[type="application/json"].
#3
in reply to:
↑ 2
@
10 years ago
Replying to programmin:
and not have the playlist blindly grab the first script tag's contents.
script[type="application/json"]
would still blindly grab the first <script>
that matched. So any theme that, for whatever reason, wanted to prepend a script with application/json
would win. This is the fault of the theme/plugin. It is breaking your playlists.
#4
@
10 years ago
Very very rarely will you see a script with application/json type, I see it's used here to store {object} that will be needed by the script, why not be specific and avoid accidental interference of any other script that happened to printout?
#5
@
10 years ago
Here's a minimal test case that actually breaks all the wp-playlist functionality in a few lines of code:
<?php /* Plugin Name: Test Description: Example causing WP bug 29383 Version: 0.1 */ add_action( 'wp_print_scripts', 'printstuff' ); function printstuff() { ?><script> (function() { var ga = document.createElement('script'); ga.type = 'text/javascript'; ga.async = true; ga.src = 'http://anywhere'; var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(ga, s); })(); </script> <?php }
#7
@
10 years ago
Still happening on RC, surprisingly this doesn't seem to happen in 3.9 because it wasn't separating wp-view into an iframe.
#11
in reply to:
↑ 9
@
10 years ago
Replying to SergeyBiryukov:
Yep, the <script>
tag supports all of the global attributes, so adding a class or id or data-* would work. Maybe even easier to use a class as jQuery caches the data-* attributes :)
#12
@
10 years ago
class
wasn't a valid attribute for <script>
in HTML4, and I wasn't sure if it's valid in HTML5. Apparently it is.
#13
@
10 years ago
Just using script[type="application/json"] should work, according to this article
http://css-tricks.com/attribute-selectors/
"Every single example above works in all modern browsers: Safari, Chrome, Firefox, Opera, and IE. Internet Explorer has perfect support for all of these down to version 7":
It would also make it clear that this "script" is actually a json it's grabbing.
Class would be ok too though.
#14
@
10 years ago
I just updated svn and it looks like this is not patched yet in wp-playlist.js Can someone please make sure this gets merged, this would be a pretty big regression.
it is looking for a script inside of
.wp-playlist
- are you shoving more<script>
s in there? If so, why?