This repository was archived by the owner on Feb 22, 2023. It is now read-only.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silently discarding all exceptions is not a fix, it's hiding the problem—and a variety of other possible problems as well, which is not okay.
seekTo
isn't doing the same up-frontisInitialized
check that other methods are doing, which seems like the actual bug here. It looks like this flow throwing is a regression from #3727 (@cc @KyleFin)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @stuartmorgan! Yes, this issue was mentioned in #3727 (comment). @jerryzhoujw let me know if there's anything I can do to help. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this change I believe you could just change line 397 to be
if (value.position == value.duration && value.isInitialized) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And unit tests could be similar to those added in https://github.com/flutter/plugins/pull/3727/files#diff-b67d4b0ee52dfea4c9c71082e12eca5459a174693c8d40577b0191d8ee498e34
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
close this since #4300 much better.