Summary: | A video element cannot enter fullscreen from PiP mode | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peng Liu <peng.liu6> | ||||||
Component: | Media | Assignee: | Peng Liu <peng.liu6> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | iPhone / iPad | ||||||||
OS: | iOS 13 | ||||||||
Attachments: |
|
Description
Peng Liu
2019-11-21 13:21:04 PST
This bug only happens on iPad. When we try to enter fullscreen from PiP, it will show the animation but the video element will be in the inline mode. Also, it will be stuck there and we cannot make further presentation mode changes (to fullscreen/PiP). Created attachment 384092 [details]
Patch
Comment on attachment 384092 [details]
Patch
This needs an API test before we can land.
Comment on attachment 384092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384092&action=review > Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm:1474 > - m_videoFullscreenModel->fullscreenModeChanged(m_currentMode.mode()); > + m_videoFullscreenModel->fullscreenModeChanged(mode); It's probably worth adding a comment about why you are making this change, IOW why does "m_currentMode.mode()" not return "mode" immediately after calling "m_currentMode.setMode(mode)" Comment on attachment 384092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384092&action=review >> Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm:1474 >> + m_videoFullscreenModel->fullscreenModeChanged(mode); > > It's probably worth adding a comment about why you are making this change, IOW why does "m_currentMode.mode()" not return "mode" immediately after calling "m_currentMode.setMode(mode)" The setMode() implementation is void setMode(HTMLMediaElementEnums::VideoFullscreenMode mode) { m_mode |= mode; } Without the update, m_currentMode will be 3 (both fullscreen and picture-in-picture). It is a strange value for the HTMLVideoElement. Comment on attachment 384092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384092&action=review >>> Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm:1474 >>> + m_videoFullscreenModel->fullscreenModeChanged(mode); >> >> It's probably worth adding a comment about why you are making this change, IOW why does "m_currentMode.mode()" not return "mode" immediately after calling "m_currentMode.setMode(mode)" > > The setMode() implementation is > void setMode(HTMLMediaElementEnums::VideoFullscreenMode mode) { m_mode |= mode; } > > Without the update, m_currentMode will be 3 (both fullscreen and picture-in-picture). It is a strange value for the HTMLVideoElement. Please add a comment to the file so someone looking at the code in the future won't change it back. Comment on attachment 384092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384092&action=review >>>> Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm:1474 >>>> + m_videoFullscreenModel->fullscreenModeChanged(mode); >>> >>> It's probably worth adding a comment about why you are making this change, IOW why does "m_currentMode.mode()" not return "mode" immediately after calling "m_currentMode.setMode(mode)" >> >> The setMode() implementation is >> void setMode(HTMLMediaElementEnums::VideoFullscreenMode mode) { m_mode |= mode; } >> >> Without the update, m_currentMode will be 3 (both fullscreen and picture-in-picture). It is a strange value for the HTMLVideoElement. > > Please add a comment to the file so someone looking at the code in the future won't change it back. Done. Created attachment 387615 [details]
Patch for landing
Comment on attachment 387615 [details] Patch for landing Clearing flags on attachment: 387615 Committed r254512: <https://trac.webkit.org/changeset/254512> |