Skip to content

Commit 7db5c0d

Browse files
amtinsedirub
authored andcommitted
fix(player): cache_.currentTime is not updated when the current time is set (videojs#8285)
Updating cache_.currentTime as soon as the currentTime is set avoids having to wait for the timeupdate event, which results in: - making cache_.currentTime more reliable - updating the progress bar on mouse up after dragging when the media is paused. See also: videojs#6232, videojs#6234, videojs#6370, videojs#6372
1 parent b148cf6 commit 7db5c0d

File tree

2 files changed

+39
-20
lines changed

2 files changed

+39
-20
lines changed

src/js/player.js

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2462,29 +2462,34 @@ class Player extends Component {
24622462
* - Nothing when setting
24632463
*/
24642464
currentTime(seconds) {
2465-
if (typeof seconds !== 'undefined') {
2466-
if (seconds < 0) {
2467-
seconds = 0;
2468-
}
2469-
if (!this.isReady_ || this.changingSrc_ || !this.tech_ || !this.tech_.isReady_) {
2470-
this.cache_.initTime = seconds;
2471-
this.off('canplay', this.boundApplyInitTime_);
2472-
this.one('canplay', this.boundApplyInitTime_);
2473-
return;
2474-
}
2475-
this.techCall_('setCurrentTime', seconds);
2476-
this.cache_.initTime = 0;
2465+
if (seconds === undefined) {
2466+
// cache last currentTime and return. default to 0 seconds
2467+
//
2468+
// Caching the currentTime is meant to prevent a massive amount of reads on the tech's
2469+
// currentTime when scrubbing, but may not provide much performance benefit after all.
2470+
// Should be tested. Also something has to read the actual current time or the cache will
2471+
// never get updated.
2472+
this.cache_.currentTime = (this.techGet_('currentTime') || 0);
2473+
return this.cache_.currentTime;
2474+
}
2475+
2476+
if (seconds < 0) {
2477+
seconds = 0;
2478+
}
2479+
2480+
if (!this.isReady_ || this.changingSrc_ || !this.tech_ || !this.tech_.isReady_) {
2481+
this.cache_.initTime = seconds;
2482+
this.off('canplay', this.boundApplyInitTime_);
2483+
this.one('canplay', this.boundApplyInitTime_);
24772484
return;
24782485
}
24792486

2480-
// cache last currentTime and return. default to 0 seconds
2481-
//
2482-
// Caching the currentTime is meant to prevent a massive amount of reads on the tech's
2483-
// currentTime when scrubbing, but may not provide much performance benefit after all.
2484-
// Should be tested. Also something has to read the actual current time or the cache will
2485-
// never get updated.
2486-
this.cache_.currentTime = (this.techGet_('currentTime') || 0);
2487-
return this.cache_.currentTime;
2487+
this.techCall_('setCurrentTime', seconds);
2488+
this.cache_.initTime = 0;
2489+
2490+
if (isFinite(seconds)) {
2491+
this.cache_.currentTime = Number(seconds);
2492+
}
24882493
}
24892494

24902495
/**

test/unit/player.test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2699,6 +2699,20 @@ QUnit.test('Should accept multiple calls to currentTime after player initializat
26992699
assert.equal(player.currentTime(), 800, 'The last value passed is stored as the currentTime value');
27002700
});
27012701

2702+
QUnit.test('Should be able to set the cache currentTime after player initialization as soon the canplay event is fired', function(assert) {
2703+
const player = TestHelpers.makePlayer({});
2704+
2705+
player.src('xyz.mp4');
2706+
player.currentTime(500);
2707+
2708+
assert.strictEqual(player.getCache().currentTime, 0, 'cache currentTime value was not changed');
2709+
2710+
this.clock.tick(100);
2711+
player.trigger('canplay');
2712+
2713+
assert.strictEqual(player.getCache().currentTime, 500, 'cache currentTime value is the one passed after initialization');
2714+
});
2715+
27022716
QUnit.test('Should fire debugon event when debug mode is enabled', function(assert) {
27032717
const player = TestHelpers.makePlayer({});
27042718
const debugOnSpy = sinon.spy();

0 commit comments

Comments
 (0)