Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

video_player: Fixed null exception when file has no width or height. #813

Merged
merged 2 commits into from
Oct 17, 2018

Conversation

quentinleguennec
Copy link
Contributor

Since the audio player plugin is not maintained we use the video player to play some mp3 files. It's working fine in iOS but crashing on Android because of a null exception. This fixes it.

@quentinleguennec quentinleguennec changed the title Fixed null exception when file as no width or height. video_player: Fixed null exception when file has no width or height. Oct 5, 2018
@robsonql
Copy link

robsonql commented Oct 7, 2018

Please accept this PR. I have the same problem.

@owenthereal
Copy link

Any update on this?

@luigi-agosti
Copy link
Contributor

It will be nice to have this fix merged

@quentinleguennec
Copy link
Contributor Author

It's been more than a week now. Can we have an ETA on when this can be merged?

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! The change looks good to me.

Can you please add an entry to the Changelog and increase the version number in the pubspec.yaml so we can release a version with this bugfix?

Can you also add a test to ensure that this does not regress in the future? Thanks!

@quentinleguennec
Copy link
Contributor Author

@goderbauer Thank you for the review! I updated the changelog and the pubspec.yaml.

I had a look at the test file and from what I understand the MethodChannel is not tested at all at the moment. This mean I would have to write a complete set of test (for everything). This should be a separate PR I think.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@goderbauer goderbauer merged commit abfb4de into flutter:master Oct 17, 2018
@goderbauer
Copy link
Member

Sadly, I am not in the list of uploaders for this plugin on pub. I've pinged @cbracken, @gspencergoog, and @bkonyi to re-publish it to pub.

@quentinleguennec
Copy link
Contributor Author

Thanks!

@gspencergoog
Copy link
Contributor

This has been published now. Version 0.7.1 is live.

@dnfield
Copy link
Contributor

dnfield commented Oct 17, 2018

@quentinleguennec - I think in this case it might still be nice to have a test where those values are passed in as null, which would fail before this patch but not fail now. We should be able to achieve that by moving the eventListener method outside of the initialize method (effectively making it public instead of private). What do you think?

@quentinleguennec
Copy link
Contributor Author

@dnfield I think there should be more tests on this plugin since it's very popular. But you are right, we need to start somewhere and that could be a good start :)

andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
…lutter#813)

Since the audio player plugin is not maintained we use the video player to play some mp3 files. It's working fine in iOS but crashing on Android because of a null exception. This fixes it.
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants