Skip to content

Commit

Permalink
Merge pull request #2090 from tf/ios-17-video-bug
Browse files Browse the repository at this point in the history
Add workaround for video scaling issue in iOS 17 below 17.4
  • Loading branch information
tf authored Mar 18, 2024
2 parents 1d98f64 + 793026a commit 9c1124b
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function Test() {

function setup() {
const audio = document.createElement('audio');

audio.setAttribute('controls', true);
audio.setAttribute('crossorigin', 'anonymous');

Expand Down Expand Up @@ -90,4 +90,3 @@ async function run(commands) {
await c;
}
}

Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import React from 'react';

import squareVideo from './100x100.mp4';
import landscapeVideo from './200x100.mp4';

export default {
title: 'Frontend/Browser Bugs',
parameters: {
percy: {skip: true}
}
}

export const safariVideoDimension = () => (
<div>
<style>
{`
.wr {width: 300px; height: 300px; }
.wr > div,
.video-js { height: 100%; }
video {
border: solid 1px red;
}
`}`
</style>
<p>
Load video 1, release it, load video 2. Observe the video being scaled
down to only cover parts of the upper part of the video element.
</p>
<div className="wr" id="container"></div>
<button onClick={() => loadVideo(0)}>Load video 1</button>
<button onClick={() => loadVideo(1)}>Load video 2</button>
<button onClick={() => release()}>Release</button>
</div>
);

const srcs = [
landscapeVideo,
squareVideo
];

let video;

function loadVideo(i) {
if (!video) {
video = document.createElement('video');
video.setAttribute('crossorigin', 'anonymous');
video.setAttribute('playsinline', true);
video.setAttribute('loop', true);
video.setAttribute('class', 'video-js wr');
}

video.src = srcs[i];

// Uncomment to fix
// video.load();

document.getElementById('container').appendChild(video);

setTimeout(() => {
video.play();
}, 1000);
}

function release() {
if (video) {
video.pause();

document.getElementById('container').removeChild(video);
}
}
49 changes: 49 additions & 0 deletions package/spec/frontend/browser/agent_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,53 @@ describe('pageflow.browser.Agent', function() {
expect(agent.matchesDesktopChrome({minVersion: 20})).toBe(false);
});
});

describe('#matchesMobileSafari', function() {
it('returns true for Safari on iPhone', function() {
var agent = new Agent(
'Mozilla/5.0 (iPhone; CPU iPhone OS 10_0_1 like Mac OS X) '+
'AppleWebKit/602.1.50 (KHTML, like Gecko) Version/10.0 Mobile/14A403 Safari/602.1'
);

expect(agent.matchesMobileSafari()).toBe(true);
});

it('returns false for desktop Safari', function() {
var agent = new Agent(
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14) ' +
'AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0 Safari/605.1.15'
);

expect(agent.matchesMobileSafari()).toBe(false);
});

describe('with osVersions option', () => {
it('returns true for matching mobile Safari', () => {
var agent = new Agent(
'Mozilla/5.0 (iPhone; CPU iPhone OS 17_3 like Mac OS X) ' +
'AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.3.1 Mobile/15E148 Safari/604.1'
);

expect(agent.matchesMobileSafari({osVersions: ['17.1', '17.2', '17.3']})).toBe(true);
});

it('returns false for other mobile Safari', () => {
var agent = new Agent(
'Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) ' +
'AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Mobile/15E148 Safari/604.1'
);

expect(agent.matchesMobileSafari({osVersions: ['17.1', '17.2', '17.3']})).toBe(false);
});

it('returns true for matching Chrome on iPhone', () => {
var agent = new Agent(
'Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) ' +
'AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/123.0.6312.52 Mobile/15E148 Safari/604.1'
);

expect(agent.matchesMobileSafari({osVersions: ['17.4']})).toBe(true);
});
});
});
});
26 changes: 18 additions & 8 deletions package/src/frontend/browser/Agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,24 @@ export const Agent = function(userAgent) {
* Returns true on iOS Safari.
* @return {boolean}
*/
matchesMobileSafari: function() {
var matchers = [/iPod/i, /iPad/i, /iPhone/i];

return (matchers.some(function(matcher) {
return userAgent.match(matcher);
}) &&
!window.MSStream) || //IE exclusion from being detected as an iOS device;
matchesiPadSafari13AndAbove();
matchesMobileSafari: function({osVersions} = {}) {
var deviceMatchers = [/iPod/i, /iPad/i, /iPhone/i];

if (osVersions) {
return (
deviceMatchers.some(matcher => userAgent.match(matcher)) &&
osVersions.some(osVersion => userAgent.includes(osVersion.replace('.', '_')))
)
}
else {
return (
matchesiPadSafari13AndAbove() ||
(
deviceMatchers.some(matcher => userAgent.match(matcher)) &&
!window.MSStream // IE exclusion from being detected as an iOS device;
)
);
}
},

/**
Expand Down
9 changes: 9 additions & 0 deletions package/src/frontend/browser/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,12 @@ browser.feature('mse and native hls support', function(has) {
browser.feature('native video player', function(has) {
return has('iphone platform');
});

browser.feature('video scaling bug fixed by load', function(has) {
// When reusing video elements for videos with different
// resolutions, Safari gets confused and scales videos incorrectly -
// drawing them to only cover a part of the element. This appears to
// not happen when the video is loaded or played immediately after
// changing the source. No longer reproducible in iOS 17.4.
return agent.matchesMobileSafari({osVersions: ['17.0', '17.1', '17.2', '17.3']});
});
5 changes: 5 additions & 0 deletions package/src/frontend/media/media.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {MediaPool, MediaType} from './MediaPool';
import BackboneEvents from 'backbone-events-standalone';
import {features} from '../features';
import {browser} from '../browser';

export const media = {
playerPool: new MediaPool({
Expand All @@ -25,6 +26,10 @@ export const media = {
options.textTrackSources.forEach(track => player.addRemoteTextTrack(track, true));
}

if (browser.has('video scaling bug fixed by load')) {
player.load();
}

return player;
}
},
Expand Down

0 comments on commit 9c1124b

Please sign in to comment.