Skip to content

Commit 4ae0b42

Browse files
authored
Merge pull request #37772 from bernhardoj/fix/37710-app-crash-when-expanding-video
Fix crash when expanding video player
2 parents e7a107b + 346b079 commit 4ae0b42

File tree

2 files changed

+22
-13
lines changed

2 files changed

+22
-13
lines changed

src/components/Attachments/AttachmentView/index.js

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ function AttachmentView({
101101
isHovered,
102102
optionalVideoDuration,
103103
}) {
104-
const {updateCurrentlyPlayingURL, currentVideoPlayerRef} = usePlaybackContext();
104+
const {updateCurrentlyPlayingURL} = usePlaybackContext();
105105
const theme = useTheme();
106106
const styles = useThemeStyles();
107107
const StyleUtils = useStyleUtils();
@@ -115,17 +115,6 @@ function AttachmentView({
115115
updateCurrentlyPlayingURL(isVideo ? source : null);
116116
}, [isFocused, isVideo, source, updateCurrentlyPlayingURL, file, isUsedInAttachmentModal]);
117117

118-
// This should ensure we clean up any video references when closing the attachment modal as these only existed here in memory during attachment preview.
119-
useEffect(
120-
() => () => {
121-
if (!isVideo) {
122-
return;
123-
}
124-
currentVideoPlayerRef.current = null;
125-
},
126-
[isVideo, currentVideoPlayerRef],
127-
);
128-
129118
const [imageError, setImageError] = useState(false);
130119

131120
useNetwork({onReconnect: () => setImageError(false)});

src/components/VideoPlayer/BaseVideoPlayer.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ function BaseVideoPlayer({
6060
const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();
6161
const isCurrentlyURLSet = currentlyPlayingURL === url;
6262
const isUploading = _.some(CONST.ATTACHMENT_LOCAL_URL_PREFIX, (prefix) => url.startsWith(prefix));
63+
const shouldUseSharedVideoElementRef = useRef(shouldUseSharedVideoElement);
6364

6465
const togglePlayCurrentVideo = useCallback(() => {
6566
videoResumeTryNumber.current = 0;
@@ -145,8 +146,27 @@ function BaseVideoPlayer({
145146
}, [currentVideoPlayerRef, handleFullscreenUpdate, handlePlaybackStatusUpdate]);
146147

147148
useEffect(() => {
149+
if (!isUploading) {
150+
return;
151+
}
152+
// If we are uploading a new video, we want to immediately set the video player ref.
148153
currentVideoPlayerRef.current = videoPlayerRef.current;
149-
}, [url, currentVideoPlayerRef]);
154+
}, [url, currentVideoPlayerRef, isUploading]);
155+
156+
useEffect(() => {
157+
shouldUseSharedVideoElementRef.current = shouldUseSharedVideoElement;
158+
}, [shouldUseSharedVideoElement]);
159+
160+
useEffect(
161+
() => () => {
162+
if (shouldUseSharedVideoElementRef.current) {
163+
return;
164+
}
165+
// If it's not a shared video player, clear the video player ref.
166+
currentVideoPlayerRef.current = null;
167+
},
168+
[currentVideoPlayerRef],
169+
);
150170

151171
// update shared video elements
152172
useEffect(() => {

0 commit comments

Comments
 (0)