FitVids.js
FitVids.js copied to clipboard
davatron5000
The automatic CSS insertion
Is there a reason why is the dynamic CSS inserted within a div and in the head? By the time the script is included (usually in footer), it's best to place it before </body> anyways.
There's another reason why this is more important now. Try this in in a normal .html file: http://pastebin.com/iGaXxgG3 and open the file in Chrome 33 (Windows/Ubuntu). There will be no text until you do some event such as resize window, refresh it etc. It's a weird bug I couldn't figure out a reason for but it was coming from the dynamic style added by FitVids.
The way the style is being dynamically inserted isn't the right way anyways. So it's better to fix it whether or not it effects a future version of Chrome.
It's due to an old IE style insertion bug. May be changed up in FitVids 2.0
I've also encountered this kind of problem, specific for Chrome. It completly ruined my layout, since I use font Icons + text, so nothing important was visible on page, as already mentioned, before resize, hover and such events. PLEASE, fix it, I really would love to use this plugin (and some premium templates also already do).
I identified this as a problem: cssStyles = ''; div.innerHTML = cssStyles;
without inserting this styles, everything works fine for me.
without inserting this styles, everything works fine for me.
If you don't insert the cssStyles then FitVids shouldn't work at all. Do you have a reduced test case (on CodePen or something similar)?
@asadkn Be sure to upgrade your FitVids from the github repo, you're a few versions behind.
Yes. I did try upgrading davatron5000. Any test case on Codepen will not work. It has to be a static page - not in an iframe - to re-create the bug. Here's a test-case you can download and try:
https://www.dropbox.com/s/yipyrujfg5ccmi9/fit-vids.html
Also, note sometimes it works on refreshes but when opening it first time, it almost never displays. Seems like a race condition to me in the layout painting.
I don't really care for < IE9 so I just changed the fitvids.js dynamic insertion method. @mkosik you can try this: https://github.com/asadkn/FitVids.js/blob/42dc0c487b6d680918f5e88bc63a69157c6dc973/jquery.fitvids.js
Alternatively, you can manually create a <style tag with id='fit-vids-style' and set the styling yourself.
@davatron5000 Yup, I know FitVid's doesn't work at all without that. (I've also updated to the latest version of fitvids, but no luck there.) I don't have a reduced test case yet, but @asadkn's version worked like a charm for me! THX
I don't really care for < IE9
Unfortunately not an option for us. Check this file: http://cl.ly/code/2n041I0a3t12 If this looks good, we can roll out a new style insertion method to IE7+.
@davatron5000 Great, I confirm your fix also works in case of my site! Looking for a new commit.
Found the bug today as well. So happy I was getting email updates on this issue or else I would've been pulling my hair out for hours. I simply moved the CSS into a dedicated file instead of inserting it dynamically.
Confirmed, the fix in 42dc0c487b works well.
For me the bug was specifically with fonts embedded via fonts.com third-party CSS link.
This was probably caused by a recent bug/fix from this recent Chrome release.
Like @mkosik said, this script is distributed to many unaware users in themes (for me it was the very popular Canvas theme from WooThemes)...
And it's not trivial to track the symptom (missing text) to this plugin... Or to get this fix distributed to users... So it would be awesome if the Chrome team could look into this.
Hi!
I have found a regression on this issue on master. Original fix in 42dc0c487b works like a charm though.
@cec There's a similar Chrome bug at the moment. https://code.google.com/p/chromium/issues/detail?id=336476
See if this fixes it: https://code.google.com/p/chromium/issues/detail?id=336476#c42
Unable to replicate regression in Chrome 35 (Dev & Canary). Can you please document bug reproduction steps?
Browser (Version):
OS:
URLs to reduced test case:
FitVids Version:
Other browsers tested (Add OK or FAIL after other browsers where you have tested this issue):
If this is part of the Chrome Font Bug (which it seems it is). It may be out of our control to fix. Then we may have to just wait for Chrome 33 to die. I'm also fine to pull in @asadkn's fix ( https://github.com/davatron5000/FitVids.js/commit/42dc0c487b) but need to confirm better browser coverage for IE<9.
Hi, sorry for the late reply, I got a deadline closing in.... @asadkn the bug you mentioned is on 34, but I have chrome 33.
@davatron5000 about your questions
OS: OS X Browser : Chrome 33 IE 9 : OK Safari : OK Fireforx : OK
I will post a reduced test case right after this deadline. I cannot give you info about coverage for IE < 9.
About waiting for 33 to die, I don't think it is the best choice, there are tons of WP themes using your plugin and lots of users suddenly get this big big font issue, which is very hard to track down.
Still I will give you the necessary info once I get my hands free.
@cec users on 33.0.1750.117 m are also experiencing the chrome bug which is mainly because of stylesheet changes in the head while loading: "Stylesheets change causes FontFaceCache::clear()."
My patch inserts the change at end of body hence it works - but it doesn't work on IE8. Perhaps a check could be added to use @davatron5000's method for IE8 but 42dc0c4 method for others.
@asadkn I agree with you, it is the fastest and safest solution. Moreover, given how different are the two browsers, it might be the only solution. @davatron5000 what do you say?
Introducing browser sniffing just to solve for a faulty version of Chrome (33) is an unfortunate reality.
I'm trying to think how it'd all work if we had 2 sub-functions like legacyAppendStyles() and appendStyles(). Might be our best solution.
Beyond that, I'm wondering if we should add a setting includeCSS: true //default. Or something. Or detect if styles already exist in the CSS (like a modernize test), then if that's true, we don't inject the styles.
Probably the earliest I can get to all of this is Monday.
Hi @davatron5000 , sorry for replying only today, got a busy family weekend :)
Today is the deadline for the site I'm working on. Tomorrow I can help you test your modifications if you need a hand.
Let me know ;)
Update: Cannot replicate the regression on Chrome 33.0.1750.117 (latest?) with the latest FitVids 1.1.0 (attached) nor the example @asadkn posted with FitVids 1.0.3.
Maybe I'm having bad luck today. Or great luck.
@cec: Can you please confirm the regression? Screenshot? Restarting the browser? Steps to replicate.
Test Files: http://cl.ly/3x0I032y0M43
@davatron5000 Make sure you don't have the google font you're testing with installed as it seems to affect the results. Testing in an incognito tab on Chrome 33.0.1750.154 m with your test files, 1.0.3 doesn't work while the latest 1.1.0 seems to work fine.
