uno
uno copied to clipboard
unoplatform
fix(mpe): show player when transport controls are disabled
GitHub Issue (If applicable): #14735
PR Type
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
PR Checklist
Please check if your PR fulfills the following requirements:
- [ ] Docs have been added/updated which fit documentation template (for bug fixes / features)
- [ ] Unit Tests and/or UI Tests for the changes have been added (for bug fixes / features) (if applicable)
- [ ] Validated PR
Screenshots Compare Test Runresults. - [ ] Contains NO breaking changes
- [ ] Associated with an issue (GitHub or internal) and uses the automatic close keywords.
- [ ] Commits must be following the Conventional Commits specification.
Other information
Internal Issue (If applicable):
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-15196/index.html
When_MediaPlayerElement_SetIsFullWindow_Check_Fullscreen seems to be failing in CI only, but the remaining tests are finally passing.
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-15196/index.html
🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-15196/index.html
It seems that the first test among the newly enabled tests always fails. Initially, CI was failing because of When_MediaPlayerElement_SetIsFullWindow_Check_Fullscreen. I disabled it and now When_MediaPlayerElement_SetSource_Check_Play is not passing (it was passing in CI when When_MediaPlayerElement_SetIsFullWindow_Check_Fullscreen was enabled). I suspect it may have to do with caching the sample video file after the first test, so the later tests are done before the timeout hits (although that would be weird as well since I've increased the timeouts to be 20secs instead of 8, it's just a 5mb file). I will test with a different host of the same video.
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-15196/index.html
🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-15196/index.html
I came back to this PR today and I think the fix is only partially correct. We should not be proceeding until SarDem/SarNum are filled in (i.e. are not zero). However, for the tests to pass (and to match WinUI):
- The video needs to load regardless of whether or not it's autoplayed. On WinUI, the video will load and fill the space (showing the first frame) even if AutoPlay is false (and the video is not played).
- In LibVLC, this is very problematic to implement. There is no method to " just load the first frame". We will need some trickery to start playing the video, but then immediately pause and go back to the first frame.
- Since most of VLC's methods don't take effect immediately (not async, they just return immediately but the effect happens later), and since VLC isn't in sync with our UI thread, actually implementing this proved to be very difficult. Every attempt I tried resulted in a race condition one way or the other.
So, we can either proceed with the PR in its current state (fill the space of the video with black and don't load anything until we actual start playing) or we can wait for a way to implement the more correct behaviour.