Added background-color to vjs-poster to remove transparent borders around scaled poster image #1868
Conversation
…ound scaled poster image
|
Interesting, thanks for the PR! I seem to remember some issue that went the other direction on this one, but it's totally escaping me. I'll look around a bit tomorrow, but otherwise this looks good to me. |
|
I still have this nagging feeling that there's some edge case we're not thinking through, but I've looked through the issues a few times and don't see anything. I might be going crazy, so lgtm unless someone else disagrees. |
|
This makes sense to me, however the poster is handled differently in older browsers that don't support background image, so we need to check if this works in IE8 too. @tjenkinson do you have IE8 available to test on? |
|
Sorry just saw your message. I don't have that browser anywhere and I just ran a test on browser stack and there result on there is a blank page which isn't particularly helpful. If anyone has the browser a url that has this extra style applied is https://embed.la1tv.co.uk/111/270?disableRedirect=1 otherwise I'll try and find a copy of xp or 7 somewhere. http://www.browserstack.com/screenshots/6058d06c6183d4c1246a9c748d334029568e3af2 |
|
Actually just emulated ie8 in ie 11 and it's crashing in js in the chapters plugin. I'll try the same locally and disable that plugin. |
|
@tjenkinson IE8 emulation is not the same. You should grab a VM from http://modern.ie to play around with. |
|
Perfect thanks will do. |
|
@tjenkinson fwiw, I have a VM and tried to load up your reduced test case. Looks like there's something unrelated to VJS causing IE8 issues :( |
|
There probably is I haven't tested anything in ie8. Maybe now's the time! |
|
Also just realised I have actually styled it differently on our site to this pull request. It's set to "cover" not "contain", so I'll need to change it back to "contain" for testing. |
|
@tjenkinson have you had a chance to test this out yet in IE8? |
|
Sorry no I haven't I downloaded the vm but haven't had chance to test it yet. |
|
Is your site meant to work in IE8? Trying to find a page I can download and then point to my pull request version.
|
|
You need to install flash in the VMs. They don't come with it. |
|
Forgot about that. Now I get this.
Built my version and all the tests pass. Just finding something to test it with. |
|
I would stick with JSBin, I think there are other issues in IE8 with our general site layout. Just remember you have to view the output in "standalone mode" so that Flash works (just remove "edit" from the URL). FWIW, I'm astonished the site doesn't crash IE8 for you... |
|
Any suggestions on where I can upload my versions where the browser can download them? I've put them here: But that's not serving up the correct mime type. |
|
@tjenkinson Welp...this one totally fell off the radar there. Would you mind re-opening this against stable so we can pull it in with the next 4.12 patch release? I can pull this into the Sass in Master. |
|
Done |
|
@mmcc this doesn't look like it was ever pulled into master. Can you confirm? |
Just found a minor issue that is when you have a cover art image that doesn't fit the player and it is scaled down, the player element is visible outside the scaled image.
This tweak sets the background-color property which means the player will always be completely covered if the poster does not fit exactly, or if the poster image cannot be loaded for some reason/whilst it's loading.
Here is what happens currently:
And here is what it looks like with the background-color property applied:
