Add seekable property #2208
Add seekable property #2208
Conversation
Expose the seekable property for the HTML tech and use duration to emulate it for Flash.
|
The tests are failing in Chrome for me but it seems unrelated to this change. I'm happy to investigate further if that's not true. |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 36b3063 (Please note that this is a fully automated comment.) |
|
Yeah, weird. Tests pass in Travis at least. Thanks for putting this together. |
|
The errors I saw were all related to the Flash tech's timeupdate interval firing after the tech had been disposed. Tracking down all the Flash techs created by tests seemed like it would be a real hairy problem. |
| let duration = 23; | ||
|
|
||
| // mock out duration | ||
| tech.el().vjs_getProperty = function(name) { |
There was a problem hiding this comment.
@dmlap Instantiating the flash tech in tests I think is what's leading to some of these vjs_getProperty jigglers. Probably because flash is either not supported or never displayed. Would it be good enough to just test the prototype function directly?
var mockFlash = {
duration: function() {
return this.duration_;
}
};
mockFlash.duration_ = 23;
let result = Flash.prototype.seekable.call(mockFlash);
// etc.
Would that cover everything we need it to?
There was a problem hiding this comment.
FYI, merged this in and going to revert all the flash tests to not create the flash object. Let me know if you have any thoughts on that.
There was a problem hiding this comment.
@heff The 4.x tests worked this way without causing the issues we're seeing in 5.x so I feel like we missed a test-infrastructure upgrade of some sort along the way. That said, I can't object too much if everything is passing now.
Expose the seekable property for the HTML tech and use duration to emulate it for Flash.