Skip to content

Commit

Permalink
feat: remove the firstplay event (#7707)
Browse files Browse the repository at this point in the history
Co-authored-by: Hugo Rodriguez <hrodriguez@brightcove.com>

BREAKING CHANGE: Removes the firstplay event. Use one('play') instead.
  • Loading branch information
hugorogz authored and misteroneill committed Nov 23, 2022
1 parent 9d832ec commit c190b21
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 90 deletions.
42 changes: 1 addition & 41 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,6 @@ class Player extends Component {
this.on(this.tech_, 'ended', (e) => this.handleTechEnded_(e));
this.on(this.tech_, 'seeking', (e) => this.handleTechSeeking_(e));
this.on(this.tech_, 'play', (e) => this.handleTechPlay_(e));
this.on(this.tech_, 'firstplay', (e) => this.handleTechFirstPlay_(e));
this.on(this.tech_, 'pause', (e) => this.handleTechPause_(e));
this.on(this.tech_, 'durationchange', (e) => this.handleTechDurationChange_(e));
this.on(this.tech_, 'fullscreenchange', (e, data) => this.handleTechFullscreenChange_(e, data));
Expand Down Expand Up @@ -1395,12 +1394,9 @@ class Player extends Component {
}

/**
* Retrigger the `loadstart` event that was triggered by the {@link Tech}. This
* function will also trigger {@link Player#firstplay} if it is the first loadstart
* for a video.
* Retrigger the `loadstart` event that was triggered by the {@link Tech}.
*
* @fires Player#loadstart
* @fires Player#firstplay
* @listens Tech#loadstart
* @private
*/
Expand All @@ -1416,9 +1412,6 @@ class Player extends Component {
// Update the duration
this.handleTechDurationChange_();

// If it's already playing we want to trigger a firstplay event now.
// The firstplay event relies on both the play and loadstart events
// which can happen in any order for a new source
if (!this.paused()) {
/**
* Fired when the user agent begins looking for media data
Expand All @@ -1427,7 +1420,6 @@ class Player extends Component {
* @type {EventTarget~Event}
*/
this.trigger('loadstart');
this.trigger('firstplay');
} else {
// reset the hasStarted state
this.hasStarted(false);
Expand Down Expand Up @@ -1653,7 +1645,6 @@ class Player extends Component {
/**
* Add/remove the vjs-has-started class
*
* @fires Player#firstplay
*
* @param {boolean} request
* - true: adds the class
Expand All @@ -1676,7 +1667,6 @@ class Player extends Component {

if (this.hasStarted_) {
this.addClass('vjs-has-started');
this.trigger('firstplay');
} else {
this.removeClass('vjs-has-started');
}
Expand Down Expand Up @@ -1856,36 +1846,6 @@ class Player extends Component {
this.trigger('seeked');
}

/**
* Retrigger the `firstplay` event that was triggered by the {@link Tech}.
*
* @fires Player#firstplay
* @listens Tech#firstplay
* @deprecated As of 6.0 firstplay event is deprecated.
* As of 6.0 passing the `starttime` option to the player and the firstplay event are deprecated.
* @private
*/
handleTechFirstPlay_() {
// If the first starttime attribute is specified
// then we will start at the given offset in seconds
if (this.options_.starttime) {
log.warn('Passing the `starttime` option to the player will be deprecated in 6.0');
this.currentTime(this.options_.starttime);
}

this.addClass('vjs-has-started');
/**
* Fired the first time a video is played. Not part of the HLS spec, and this is
* probably not the best implementation yet, so use sparingly. If you don't have a
* reason to prevent playback, use `myPlayer.one('play');` instead.
*
* @event Player#firstplay
* @deprecated As of 6.0 firstplay event is deprecated.
* @type {EventTarget~Event}
*/
this.trigger('firstplay');
}

/**
* Retrigger the `pause` event that was triggered by the {@link Tech}.
*
Expand Down
49 changes: 0 additions & 49 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1055,55 +1055,6 @@ QUnit.test('should register players with generated ids', function(assert) {
player.dispose();
});

QUnit.test('should not add multiple first play events despite subsequent loads', function(assert) {
assert.expect(1);

const player = TestHelpers.makePlayer({});

player.on('firstplay', function() {
assert.ok(true, 'First play should fire once.');
});

// Checking to make sure onLoadStart removes first play listener before adding a new one.
player.tech_.trigger('loadstart');
player.tech_.trigger('loadstart');
player.tech_.trigger('play');
player.dispose();
});

QUnit.test('should fire firstplay after resetting the player', function(assert) {
const player = TestHelpers.makePlayer({});

let fpFired = false;

player.on('firstplay', function() {
fpFired = true;
});

// init firstplay listeners
player.tech_.trigger('loadstart');
player.tech_.trigger('play');
assert.ok(fpFired, 'First firstplay fired');

// reset the player
player.tech_.trigger('loadstart');
fpFired = false;
player.tech_.trigger('play');
assert.ok(fpFired, 'Second firstplay fired');

// the play event can fire before the loadstart event.
// in that case we still want the firstplay even to fire.
player.tech_.paused = function() {
return false;
};
fpFired = false;
// reset the player
player.tech_.trigger('loadstart');
// player.tech_.trigger('play');
assert.ok(fpFired, 'Third firstplay fired');
player.dispose();
});

QUnit.test('should remove vjs-has-started class', function(assert) {
assert.expect(3);

Expand Down

0 comments on commit c190b21

Please sign in to comment.