Closed (fixed)
Project:
MyTube
Version:
6.x-1.0
Component:
Code
Priority:
Minor
Category:
Bug report
Assigned:
Reporter:
Created:
8 May 2013 at 15:20 UTC
Updated:
21 Oct 2016 at 19:44 UTC
Jump to comment: Most recent, Most recent file
I'm seeing script tags in multiple places on a page when viewing page source. I believe inline script tags and code should only be placed within the field of the node of where the embedded/iframe/object code is located. (We were running strip_tags on a template file for another field on that page and saw "var mytubes = new Array(1)..." as text on the page itself).
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | mytube-script_embed_specified_fields-1990202-2.patch | 598 bytes | awakash |
| #1 | script_embed_specified_fields-1990202-1.patch | 598 bytes | awakash |
Comments
Comment #1
awakash commentedComment #2
awakash commentedFilename corrected this time.
Comment #3
l@va commentedWhen I first read your comment, it looked like you were saying I needed to have the
mytubesarray defined in the head instead of the output text, but having looked at your patch it seems that is not the issue you're addressing (which would've caused themytubesarray to print only the embeds from the first page to hit Drupal's cache, and no other pages).I vaguely remember a reason for defining the
mytubesarray in this manner, but it was a while ago. If I remember correctly, it ended up only defining the first element of themytubesarray, so if the page had multiple processed videos only the first would launch when clicked. MyTube needs to run completely through the input and create the last array, or else not all embeds will be processed.Your proposed patch, which checks if the string contains the text "mytubetrigger", assumes the user will not include that string anywhere in their input (and will break if someone does). As a rule, you should not make assumptions about what users will or will not submit. Therefore, I cannot accept it. If you have a better idea on how to only print one instance of the array, feel free to submit another patch; I'm all ears.
I'm changing the priority to minor because this has no impact on functionality. It's just slightly messy if you view the page source and trace the javascript by hand.
Comment #4
mfbI believe this issue was resolved by #2421715: Current embed insertion is not depdendable