Fix/ios preroll #115
Fix/ios preroll #115
Conversation
| @@ -535,14 +553,17 @@ class VR extends Plugin { | |||
|
|
|||
| if (this.controls3d) { | |||
| this.controls3d.dispose(); | |||
| this.controls3d = null; | |||
There was a problem hiding this comment.
mostly we needed this change, since we check to see if this variable exists before adding orbit orientation controls, and even with dispose it will still exist.
|
So, should we have the reset code for ads here or should the ad plugins know to reset the VR plugin when enabled? Thoughts @incompl ? |
|
Yeah we probably want to set it whenever |
|
We have been trying to keep all ad-related code contained. I'm not sure where it is necessarily better, though. |
src/plugin.js
Outdated
| @@ -83,6 +83,27 @@ class VR extends Plugin { | |||
|
|
|||
| this.setProjection(this.options_.projection); | |||
|
|
|||
| // any time the video element is recycled for ads | |||
| // we have to reset the vr state and re-init after ad | |||
| this.on(player, 'adstart', () => window.setTimeout(() => { | |||
There was a problem hiding this comment.
we have to wait 1 tick after adstart so that we can tell if the video element was recycled or not
There was a problem hiding this comment.
we should use player.setTimeout
src/plugin.js
Outdated
| videojs.log('VR: ' + msg); | ||
| } else { | ||
| videojs.log('VR: ', msg); | ||
| } |
There was a problem hiding this comment.
Can't you just do videojs.log('VR: ', msg); without the code fork? Should be the same end result, no?
src/plugin.js
Outdated
| @@ -83,6 +83,27 @@ class VR extends Plugin { | |||
|
|
|||
| this.setProjection(this.options_.projection); | |||
|
|
|||
| // any time the video element is recycled for ads | |||
| // we have to reset the vr state and re-init after ad | |||
| this.on(player, 'adstart', () => window.setTimeout(() => { | |||
There was a problem hiding this comment.
we should use player.setTimeout
src/plugin.js
Outdated
| @@ -240,7 +261,11 @@ class VR extends Plugin { | |||
| } | |||
|
|
|||
| msgs.forEach((msg) => { | |||
| videojs.log(msg); | |||
| if (typeof msg === 'string') { | |||
There was a problem hiding this comment.
why not always do videojs.log('VR:', msg)?
|
QA LGTM |
Reset needs to null out variables and dispose, just dispose won't actually reset the plugin.
IOS has a special case for many ad plugins that we have to handle.