Restore video tag attributes on tech change #1369
Conversation
|
Thanks for this! Digging in now. |
| * @private | ||
| */ | ||
| vjs.setElementAttributes = function(el, attributes){ | ||
| var keys = Object.keys(attributes); |
There was a problem hiding this comment.
I believe Object.keys will error in ie8. You can use vjs.obj.each instead.
| @@ -32,6 +32,8 @@ vjs.Player = vjs.Component.extend({ | |||
| init: function(tag, options, ready){ | |||
| this.tag = tag; // Store the original tag used to set options | |||
|
|
|||
| this.tagAttributes = (tag) ? vjs.getAttributeValues(tag) : {}; // Store the tag attributes used to restore html5 tech | |||
There was a problem hiding this comment.
Would it work to store this later in the file where we already run getAttributeValues?
https://github.com/dcharbonnier/video.js/blob/feature/video-attr-restore/src/js/player.js#L134
There was a problem hiding this comment.
Let me try, that can solve two concerns but the code coverage looks quite low on this project so I'm afraid of such changes.
There was a problem hiding this comment.
getTagSettings is a static method (as much as javascript permit it), accessing an attribute of the instance on this method seams strange to me, even if the reality is that we only use this method on the constructor. I don't think it's wrong but to make it right this imply more work, rename the method, remove the arguments, do you have a convention for "private" methods ?
|
@dcharbonnier I wanted to get this merged in for tomorrow's release, but check out this change log and let me know what you think. |
|
good for me, thanks |
New attempt to fix this issue #1367 related to the webkit-playsinline attribute missing.