Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Rename _isPlaying to _playbackStarted. #144

Merged
merged 1 commit into from
Mar 17, 2015
Merged

Conversation

bclwhitaker
Copy link
Contributor

Revert removal of _isPlaying check, so that pause will not trigger if the video has ended and pause() is called. Still make sure that we capture the paused state before we call pause though.

…heck, so that pause will not trigger if the video has ended and pause() is called.
@gkatsev
Copy link
Member

gkatsev commented Mar 17, 2015

LGTM.

@@ -291,7 +291,7 @@ package com.videojs.providers{
public function pause():void{
var alreadyPaused = _isPaused;
_ns.pause();
if(!alreadyPaused){
if(_playbackStarted && !alreadyPaused){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the important bit.

@mmcc
Copy link
Member

mmcc commented Mar 17, 2015

👍

@heff
Copy link
Member

heff commented Mar 17, 2015

Looks good to me. Could this affect other ad plugins some how?

@gkatsev
Copy link
Member

gkatsev commented Mar 17, 2015

The new version of the ad plugin needs these changes.

@bclwhitaker
Copy link
Contributor Author

Ad plugins using the current version of contrib-ads should be positively impacted as well with this change (and Gary's which we pulled in earlier). It takes care of an issue with preroll behavior and autoplay settings which were causing the swf to report the paused state incorrectly.

heff added a commit that referenced this pull request Mar 17, 2015
Rename _isPlaying to _playbackStarted.
@heff heff merged commit a6adcbf into videojs:master Mar 17, 2015
heff added a commit to heff/video-js-swf that referenced this pull request Mar 17, 2015
@bclwhitaker bclwhitaker deleted the pauseEvent branch April 23, 2015 12:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants