search

davatron5000 / FitVids.js

A lightweight, easy-to-use jQuery plugin for fluid width video embeds.
http://fitvidsjs.com
4.77k stars 988 forks source link

iFrame Embed Swapping/Duplicating #33

Closed jonathanmoore closed 12 years ago

jonathanmoore commented 12 years ago

Recently we started noticing an issue with FitVids where it will cause all the iFrame embeds on the page to swap out for one of the random embeds duplicating the video across the page. In testing we can reproduce this 100% of the time in the latest versions of Chrome (16.0.912.77) and Safari (5.1.2). Oddly Firefox 6 & 8 do not have the issue, but Firefox 3.6 behaves the same way.

jsFiddle - http://jsfiddle.net/nAXXY/5/ Five embeds (Vimeo and Youtube), simple CSS, jQuery 1.7.1 and latest FitVids.js

Steps to reproduce:

  1. Load up http://jsfiddle.net/nAXXY/5/
  2. Click on any link taking you to a new page or type in a new URL
  3. Hit back on your browser
  4. Refreshing the page will correct the issue

Video demo - http://cloud.jonathanmoore.com/Ddsv

jonathanmoore commented 12 years ago

Just to test things out a bit further I even tried changing the target from $('article') to the parent $('#main'). http://jsfiddle.net/nAXXY/7/

We're also noticing the same issue on the tests.html file included with FitVids, with the exception of the Viddler embeds.

jonathanmoore commented 12 years ago

Digging in a bit further it seems like the issue narrows down to the jQuery wrap() call. Here's an example where I removed virtually all of the FitVids functionality simply leaving a wrap() call on each video.

http://jsfiddle.net/nAXXY/17/

davatron5000 commented 12 years ago

Great work! What a weird development. I'll take a look at this tonight. Will see if replacing wrap() with good old fashioned append() will work... otherwise, not sure what the solution would officially be.

Looks like the jQuery.wrap() function uses .clone(), so there must be something that's getting lost in translation there.

Ooops, close fail

jonathanmoore commented 12 years ago

This certainly seems like an issue with jQuery's wrap() and innerWrap(). I'm going to do a bit more testing before I submit a bug ticket to jQuery.

Here's a stripped down example that still has the issue with basic iFrames loading up URLs and jQuery innerWrap - http://jsfiddle.net/Kj8vb/4/

Removing the iframe HTML, wrapping and then adding the HTML back in still has the same effect - http://jsfiddle.net/Kj8vb/5/

davatron5000 commented 12 years ago

Ya. I've been looking into commits between 1.7 and 1.7.1 and it looks like they've changed a lot of how wrap() and clone() work. And they're definitely using some kind of cache called rnoshimcache that I know nothing about.

chriscoyier commented 12 years ago

Confirmed that it's doing this on Chrome 17.0.963.44 beta and Firefox 3.6.

SO WACKY.

jonathanmoore commented 12 years ago

Ok. I spent a bit more time with this issue and realized that this issue goes beyond jQuery and is related to how Webkit handles appendChild. The reason why it works in Firefox is because they only recently fixed this lingering issue.

Here is an old school Javascript example with appendChild to wrap iFrame - http://jsfiddle.net/Kj8vb/17/

With a bit more testing I noticed that this issue goes away when you add an id to each of the iFrames - http://jsfiddle.net/Kj8vb/19/

In the end I was able to fix the issue in FitVids by adding an id if one does not exist. http://jsfiddle.net/nAXXY/42/ I'll submit a pull request, although I am certain that there's a more elegant solution.

davatron5000 commented 12 years ago

Great work! I think this solution is just perfect. Browser bugs would be ridiculously slow to fix. So this is wonderful.

chriscoyier commented 12 years ago

I was going to suggest the ID be "chicky-chicky-woo-woo.gif" + Math.random(99999);

davatron5000 commented 12 years ago

:+1::+1::+1::+1::+1::+1:

tgrnwd commented 10 years ago

I seem to be experiencing this issue again (only in Chrome) @ http://leankit.com/product/

I've tried adding unique ID's directly to the markup, both the container and the iframe: no dice.

Also tried modifying the target from $('#content') to $('.fitvid') and adding .fitvid to the relevant container classes: no dice.

Safari and Firefox don't have this problem.

This is what I currently have: 

$(document).ready(function(){
  $("#content").fitVids();
});
kenhowardpdx commented 10 years ago

I can confirm this behavior in Chrome Version 35.0.1916.153

To clarify, there are two videos on this page. Scroll down to see the second video that has pie charts and graphs. Navigate to another page and then use your browser to go back to the previous page. Now there will be two instances of one of the two videos and the other will be missing.

@tgrnwd - I doubt this is the issue, but I noticed you are using $(document).ready... back-to-back in your script. You don't need to wrap your script in $(document).ready if you are loading your script in the footer, below all of the DOM elements. Also, that's a lot of JS for one page. Does the page require that much?

davatron5000 commented 10 years ago

Unable to replicate in Chrome 37.0.2024.3

It could be temporal (2~6 week) bug with Chrome or it could be lazyloading of the iframe?

h=function(t,n){var r=t.find("img, iframe").length;if(0==r)return n(),void 0;var i=0;t.find("img, iframe").each(function(){e(this).one("load",function(){++i==r&&n()}).each(function(){this.complete&&e(this).load()})})}

↑ Minified JS that does something to each image and iframe on load. Unable to decipher what it means, but it appears content is lazyloaded at some point. Trying to replicate, at one point, I hit the backbutton and got a large_spinner.gif and no video. That might explain why 2 vides were on the page.

Can we get a reduced test case on CodePen or somewhere that we can operate on? Pretty difficult to dissect from this point.

zdenekvecera commented 10 years ago

Hi, described issue (http://jsfiddle.net/Kj8vb/5/) also appears to me. Testing the Google Chrome (36.0.1985.125, 38.0.2100.0) and Opera (22.0.1471.70, 24.0.1558.3). It is very annoying.

kenhowardpdx commented 10 years ago

@zdenekvecera - Your Reduced Test Case ironically revealed the issue is not with FitVids, but more likely with Chrome (Chromium) itself.

I took your example, which didn't include the fitvids.js plugin, and added a link to google to test navigating away from the page and used the browsers back button to go back to the page and let it load from browser cache. http://fiddle.jshell.net/Kj8vb/20/show/ (using 36.0.1985.125 & 38.0.2100.0 canary 64-bit)

The issue didn't reveal itself until I went back and forth 5 or 6 times.

@davatron5000, @chriscoyier - any chance you can verify? And then close this non-bug (if it indeed is a non-bug) issue.

kenhowardpdx commented 10 years ago

Found a similar thread here: https://vimeo.com/forums/topic:69849

zdenekvecera commented 10 years ago

@kenhowardpdx I also think that it is not the fault FitVids. I have a problem with pure iframe without reference to FitVids too.

I reported to Chrome and Opera:

https://code.google.com/p/chromium/issues/detail?can=2&q=iframe&colspec=ID%20Pri%20M%20Iteration%20ReleaseBlock%20Cr%20Status%20Owner%20Summary%20OS%20Modified&id=395533&thanks=395533&ts=1405933036

DNA-23745@bugs.opera.com + http://blogs.opera.com/desktop/2014/07/opera-next-23-0-1522-58-update/#comment-1495139841

forestkelley commented 9 years ago

Has a solution arisen? I'm having the same issue with Chrome 41.0.2272.104 (64-bit).