Skip to content

Commit

Permalink
fix: Guard against Safari adding native controls after fiullscreen
Browse files Browse the repository at this point in the history
  • Loading branch information
mister-ben committed Feb 4, 2022
1 parent eeda26f commit 13043fc
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 3 deletions.
5 changes: 4 additions & 1 deletion src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -2124,7 +2124,10 @@ class Player extends Component {
handleTechFullscreenChange_(event, data) {
if (data) {
if (data.nativeIOSFullscreen) {
this.toggleClass('vjs-ios-native-fs');
this.addClass('vjs-ios-native-fs');
this.tech_.one('webkitendfullscreen', () => {
this.removeClass('vjs-ios-native-fs');
});
}
this.isFullscreen(data.isFullscreen);
}
Expand Down
7 changes: 5 additions & 2 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,11 +641,14 @@ class Html5 extends Tech {

const endFn = function() {
this.trigger('fullscreenchange', { isFullscreen: false });
// Safari will sometimes set contols on the videoelement when existing fullscreen.
if (this.el_.controls && !this.options_.nativeControlsForTouch && this.controls()) {
this.el_.controls = false;
}
};

const beginFn = function() {
if ('webkitPresentationMode' in this.el_ &&
this.el_.webkitPresentationMode !== 'picture-in-picture') {
if ('webkitPresentationMode' in this.el_ && this.el_.webkitPresentationMode !== 'picture-in-picture') {
this.one('webkitendfullscreen', endFn);

this.trigger('fullscreenchange', {
Expand Down
48 changes: 48 additions & 0 deletions test/unit/player-fullscreen.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-env qunit */
import Player from '../../src/js/player.js';
import Html5 from '../../src/js/tech/html5.js'; // eslint-disable-line no-unused-vars
import TestHelpers from './test-helpers.js';
import sinon from 'sinon';
import window from 'global/window';
Expand All @@ -21,6 +22,19 @@ const FullscreenTestHelpers = {
};

return player;
},
fakeSafariVideoEl() {
const testEl = document.createElement('video');

if (!('webkitPresentationMode' in testEl)) {
testEl.webkitPresentationMode = 'test';
}

if (!('webkitDisplayingFullscreen' in testEl)) {
testEl.webkitDisplayingFullscreen = false;
}

return testEl;
}
};

Expand Down Expand Up @@ -265,6 +279,40 @@ QUnit.test('fullscreenchange event from Html5 should change player.isFullscreen_
player.dispose();
});

QUnit.test('fullscreenchange event from Html5 should guard against Safari showing double controls', function(assert) {
const player = FullscreenTestHelpers.makePlayer(undefined, {techOrder: ['html5']}, FullscreenTestHelpers.fakeSafariVideoEl());
const html5 = player.tech(true);

html5.trigger('webkitbeginfullscreen');

assert.ok(player.isFullscreen(), 'player.isFullscreen_ should be true');

player.tech_.el_.controls = true;

html5.trigger('webkitendfullscreen');

assert.ok(!player.tech_.el_.controls, 'el controls should be false');

player.dispose();
});

QUnit.test('Safari leaving fullscreen should retain controls with nativeControlsForTouch', function(assert) {
const player = FullscreenTestHelpers.makePlayer(undefined, {techOrder: ['html5'], nativeControlsForTouch: true}, FullscreenTestHelpers.fakeSafariVideoEl());
const html5 = player.tech(true);

html5.trigger('webkitbeginfullscreen');

assert.ok(player.isFullscreen(), 'player.isFullscreen_ should be true');

player.tech_.el_.controls = true;

html5.trigger('webkitendfullscreen');

assert.ok(player.tech_.el_.controls, 'el controls should be true');

player.dispose();
});

QUnit.test('fullscreenerror event from Html5 should pass through player', function(assert) {
const player = FullscreenTestHelpers.makePlayer(false);
const html5 = player.tech(true);
Expand Down

0 comments on commit 13043fc

Please sign in to comment.