From 0e842d4f9577d3485825841176c0d7e48f42f6ee Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 1 Nov 2023 03:32:19 -0700 Subject: [PATCH 1/4] [video_player_web] Listen to loadedmetadata as a possible initialization event. --- .../integration_test/video_player_test.dart | 17 ++++++++++ .../lib/src/video_player.dart | 33 ++++++++++++++----- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/packages/video_player/video_player_web/example/integration_test/video_player_test.dart b/packages/video_player/video_player_web/example/integration_test/video_player_test.dart index 831b5a38876..01f8e2f3435 100644 --- a/packages/video_player/video_player_web/example/integration_test/video_player_test.dart +++ b/packages/video_player/video_player_web/example/integration_test/video_player_test.dart @@ -197,6 +197,23 @@ void main() { expect(events[0].eventType, VideoEventType.initialized); }); + // Issue: https://github.com/flutter/flutter/issues/137023 + testWidgets('loadedmetadata dispatches initialized', + (WidgetTester tester) async { + video.dispatchEvent(html.Event('loadedmetadata')); + video.dispatchEvent(html.Event('loadedmetadata')); + + final Future> stream = timedStream + .where((VideoEvent event) => + event.eventType == VideoEventType.initialized) + .toList(); + + final List events = await stream; + + expect(events, hasLength(1)); + expect(events[0].eventType, VideoEventType.initialized); + }); + // Issue: https://github.com/flutter/flutter/issues/105649 testWidgets('supports `Infinity` duration', (WidgetTester _) async { setInfinityDuration(video); diff --git a/packages/video_player/video_player_web/lib/src/video_player.dart b/packages/video_player/video_player_web/lib/src/video_player.dart index 75322a04c16..a225d307f24 100644 --- a/packages/video_player/video_player_web/lib/src/video_player.dart +++ b/packages/video_player/video_player_web/lib/src/video_player.dart @@ -58,7 +58,9 @@ class VideoPlayer { /// This method sets the required DOM attributes so videos can [play] programmatically, /// and attaches listeners to the internal events from the [html.VideoElement] /// to react to them / expose them through the [VideoPlayer.events] stream. - void initialize() { + void initialize({ + String? src, + }) { _videoElement ..autoplay = false ..controls = false; @@ -68,14 +70,11 @@ class VideoPlayer { // This property is not exposed through dart:html so we use the // HTML Boolean attribute form (when present with any value => true) // See: https://developer.mozilla.org/en-US/docs/Glossary/Boolean/HTML - _videoElement.setAttribute('playsinline', 'true'); + _videoElement.setAttribute('playsinline', true); - _videoElement.onCanPlay.listen((dynamic _) { - if (!_isInitialized) { - _isInitialized = true; - _sendInitialized(); - } - }); + _videoElement.onCanPlay.listen(_onVideoElementInitialization); + // Needed for Safari iOS 17, which may not send `canplay`. + _videoElement.onLoadedMetadata.listen(_onVideoElementInitialization); _videoElement.onCanPlayThrough.listen((dynamic _) { setBuffering(false); @@ -122,6 +121,10 @@ class VideoPlayer { setBuffering(false); _eventController.add(VideoEvent(eventType: VideoEventType.completed)); }); + + if (src != null) { + _videoElement.src = src; + } } /// Attempts to play the video. @@ -252,6 +255,20 @@ class VideoPlayer { _videoElement.load(); } + // Handler to mark (and broadcast) when this player [_isInitialized]. + // + // (Used as a JS event handler for "canplay" and "loadedmetadata") + // + // This function can be called multiple times by different JS Events, but it'll + // only broadcast an "initialized" event the first time it's called, and ignore + // the rest of the calls. + void _onVideoElementInitialization(Object? _) { + if (!_isInitialized) { + _isInitialized = true; + _sendInitialized(); + } + } + // Sends an [VideoEventType.initialized] [VideoEvent] with info about the wrapped video. void _sendInitialized() { final Duration? duration = From 98974d891cafaea565bcb373c76c01d5c1cecb18 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 1 Nov 2023 03:32:53 -0700 Subject: [PATCH 2/4] [video_player_web] Ensure src of the video tag is set after everything else is configured. --- .../video_player/video_player_web/lib/video_player_web.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player_web/lib/video_player_web.dart b/packages/video_player/video_player_web/lib/video_player_web.dart index 77b3cca2804..9fe07d1204e 100644 --- a/packages/video_player/video_player_web/lib/video_player_web.dart +++ b/packages/video_player/video_player_web/lib/video_player_web.dart @@ -75,7 +75,6 @@ class VideoPlayerPlugin extends VideoPlayerPlatform { final VideoElement videoElement = VideoElement() ..id = 'videoElement-$textureId' - ..src = uri ..style.border = 'none' ..style.height = '100%' ..style.width = '100%'; @@ -85,7 +84,9 @@ class VideoPlayerPlugin extends VideoPlayerPlatform { 'videoPlayer-$textureId', (int viewId) => videoElement); final VideoPlayer player = VideoPlayer(videoElement: videoElement) - ..initialize(); + ..initialize( + src: uri, + ); _videoPlayers[textureId] = player; From 87823c2081405b66687657f4313fca73dade7e4b Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 1 Nov 2023 03:33:12 -0700 Subject: [PATCH 3/4] Changelog and version. --- packages/video_player/video_player_web/CHANGELOG.md | 6 ++++++ packages/video_player/video_player_web/pubspec.yaml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/video_player/video_player_web/CHANGELOG.md b/packages/video_player/video_player_web/CHANGELOG.md index 5e02936f971..62d0536cd8e 100644 --- a/packages/video_player/video_player_web/CHANGELOG.md +++ b/packages/video_player/video_player_web/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.1.2 + +* Listens to `loadedmetadata` as an event that marks that initialization is + complete. (Fixes playback in Safari iOS 17). +* Sets the `src` of the underlying video element after every other attribute. + ## 2.1.1 * Ensures that the `autoplay` attribute of the underlying video element is set diff --git a/packages/video_player/video_player_web/pubspec.yaml b/packages/video_player/video_player_web/pubspec.yaml index 19a7aac202a..8b312aef385 100644 --- a/packages/video_player/video_player_web/pubspec.yaml +++ b/packages/video_player/video_player_web/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player_web description: Web platform implementation of video_player. repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_web issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.1.1 +version: 2.1.2 environment: sdk: ">=3.1.0 <4.0.0" From 1230c9943b28269bf6476e01f0c9d71585b693d3 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 1 Nov 2023 13:26:07 -0700 Subject: [PATCH 4/4] Document why src is nullable and needs to be set last. --- .../video_player_web/lib/src/video_player.dart | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/video_player/video_player_web/lib/src/video_player.dart b/packages/video_player/video_player_web/lib/src/video_player.dart index a225d307f24..4adb2e1e866 100644 --- a/packages/video_player/video_player_web/lib/src/video_player.dart +++ b/packages/video_player/video_player_web/lib/src/video_player.dart @@ -58,6 +58,14 @@ class VideoPlayer { /// This method sets the required DOM attributes so videos can [play] programmatically, /// and attaches listeners to the internal events from the [html.VideoElement] /// to react to them / expose them through the [VideoPlayer.events] stream. + /// + /// The [src] parameter is the URL of the video. It is passed in from the plugin + /// `create` method so it can be set in the VideoElement *last*. This way, all + /// the event listeners needed to integrate the videoElement with the plugin + /// are attached before any events start firing (events start to fire when the + /// `src` attribute is set). + /// + /// The `src` parameter is nullable for testing purposes. void initialize({ String? src, }) { @@ -122,6 +130,8 @@ class VideoPlayer { _eventController.add(VideoEvent(eventType: VideoEventType.completed)); }); + // The `src` of the _videoElement is the last property that is set, so all + // the listeners for the events that the plugin cares about are attached. if (src != null) { _videoElement.src = src; }