From 7a072af2135587f8b09e845b07a3dd71452a6d08 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Wed, 15 Jan 2025 16:55:10 -0500 Subject: [PATCH] fix(replay): Mobile replays look off when orientation changes (#83510) We showed the previous video to fix the video flickering in Safari. However, if the orientation has changed, you can see both replay videos. We can't show the previous replay in this case. This also fixes an issue where video clips other than the one that's seeked to are firing off the `seeking` event, and if that clip has a different orientation, it causes the visible replay clip to have the wrong dimensions. Before: https://github.com/user-attachments/assets/51e434aa-7de2-4eaf-b991-cf5dad144974 After: https://github.com/user-attachments/assets/bf1f41da-21a1-4865-90d2-f67ad0a679e8 Unfortunately on Safari, if the clips have gaps in them and the orientation has changed, these gaps will be visible. There is nothing feasible that we can do on the player side to fix this. After on Safari: https://github.com/user-attachments/assets/8dc124a0-f5cf-444d-b27f-6341ac010196 Fixes https://github.com/getsentry/sentry/issues/83425 --------- Co-authored-by: Ryan Albrecht Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> --- static/app/components/replays/player/styles.tsx | 1 + static/app/components/replays/videoReplayer.tsx | 12 ++++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/static/app/components/replays/player/styles.tsx b/static/app/components/replays/player/styles.tsx index eafd8f48bc12bd..c6b7af2c692e11 100644 --- a/static/app/components/replays/player/styles.tsx +++ b/static/app/components/replays/player/styles.tsx @@ -3,6 +3,7 @@ import {css, type Theme} from '@emotion/react'; // Base styles, to make the Replayer instance work export const baseReplayerCss = css` .replayer-wrapper { + /* Videos have z-index, so we need a z-index here so user interactions is on top of the video */ z-index: 1000000; user-select: none; } diff --git a/static/app/components/replays/videoReplayer.tsx b/static/app/components/replays/videoReplayer.tsx index b11c8cc016f5b2..cc441b2a6002f0 100644 --- a/static/app/components/replays/videoReplayer.tsx +++ b/static/app/components/replays/videoReplayer.tsx @@ -173,7 +173,10 @@ export class VideoReplayer { const handleSeeking = event => { // Centers the video when seeking (and video is not playing) - this._callbacks.onLoaded!(event); + // Only call this for the segment that's being seeked to + if (index === this._currentIndex) { + this._callbacks.onLoaded!(event); + } }; el.addEventListener('ended', handleEnded); @@ -408,7 +411,12 @@ export class VideoReplayer { for (const [index, videoElem] of this._videos) { // On safari, some clips have a ~1 second gap in the beginning so we also need to show the previous video to hide this gap - if (index === (this._currentIndex || 0) - 1) { + // Edge case: Don't show previous video if size is different (eg. orientation changes) + if ( + index === (this._currentIndex || 0) - 1 && + videoElem.videoHeight === nextVideo.videoHeight && + videoElem.videoWidth === nextVideo.videoWidth + ) { if (videoElem.duration) { // we need to set the previous video to the end so that it's shown in case the next video has a gap at the beginning // setting it to the end of the video causes the 'ended' bug in Chrome so we set it to 1 ms before the video ends