This repository has been archived by the owner on Feb 22, 2023. It is now read-only.
[video_player, video_player_web] PlatformException fix #2483
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Bump version and update CHANGELOG.
…rror`. * Return the `videoElement.play()` Future from the implementation of the `play()` method, so we don't end up with unhandled Futures in tests or otherwise. * Update README * Bump version and CHANGELOG
|
/cc @cbenhagen does this make sense to you? |
harryterkelsen
approved these changes
Jan 24, 2020
packages/video_player/video_player_web/test/video_player_web_test.dart
Outdated
Show resolved
Hide resolved
* Funnel the DomException to the eventController as an error.
harryterkelsen
approved these changes
Jan 24, 2020
packages/video_player/video_player_web/test/video_player_web_test.dart
Outdated
Show resolved
Hide resolved
|
(I'll merge and release this on monday, all is green now!) |
|
Published: |
WoodyGuo
pushed a commit
to liuwei1130/plugins
that referenced
this pull request
Feb 10, 2020
…convert them to PlatformExceptions (flutter#2483) video_player_web: * Add a `PlatformException` to the event Stream `onError`, instead of the Event coming straight from the DOM. * Handle videoElement.play() rejection. Funnel that as a PlatformException to the event Stream as well. * Update documentation about web-specific _quirks and features_. video_player: * Add documentation about the web platform.
WoodyGuo
pushed a commit
to liuwei1130/plugins
that referenced
this pull request
Feb 10, 2020
…convert them to PlatformExceptions (flutter#2483) video_player_web: * Add a `PlatformException` to the event Stream `onError`, instead of the Event coming straight from the DOM. * Handle videoElement.play() rejection. Funnel that as a PlatformException to the event Stream as well. * Update documentation about web-specific _quirks and features_. video_player: * Add documentation about the web platform.
FlutterSu
pushed a commit
to FlutterSu/flutter-plugins
that referenced
this pull request
Nov 20, 2020
…convert them to PlatformExceptions (flutter#2483) video_player_web: * Add a `PlatformException` to the event Stream `onError`, instead of the Event coming straight from the DOM. * Handle videoElement.play() rejection. Funnel that as a PlatformException to the event Stream as well. * Update documentation about web-specific _quirks and features_. video_player: * Add documentation about the web platform.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When
videoElement.onErrortriggers, we create aPlatformExceptionwith data coming from thevideoElement.errorobject, and add it as an error to theeventControllerStream.Before this change, the
Eventpassed in toonErrorwas passed directly to the Stream (but listeners at the other end are expecting a PlatformException.)The PR also returns the Future coming from
videoElement.play()up the stack, so it can be handled properly (especially when it errors). This was causing issues in tests, when that unhandled Future would error unexpectedly.This also updates the documentation for both
video_playerandvideo_player_webwith regards to installation and browser support.Related Issues
Fixes: flutter/flutter#48884
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
Note: there's a minor API change in the play() method of the private _VideoPlayer class. The change seems to be backwards-compatible.