Summary: | [WPE] Implement fullscreen in WPEView | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Imanol Fernandez <ifernandez> | ||||||||||
Component: | WPE WebKit | Assignee: | Imanol Fernandez <ifernandez> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aperez, bugs-noreply, lmoura | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: |
https://github.com/WebPlatformForEmbedded/libwpe/pull/86 https://github.com/Igalia/cog/pull/329 https://bugs.webkit.org/show_bug.cgi?id=229380 |
||||||||||||
Bug Depends on: | 228793 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Imanol Fernandez
2021-07-14 07:27:32 PDT
Created attachment 433505 [details]
Patch
wip
Created attachment 433792 [details]
Patch
Update to latest libwpe API
As for the WPE build failure, we will need to land the new API in libwpe first, and then update the Flatpak SDK to build a newer revision of libwpe. We should do that in a separate patch that only touches the SDK build recipes :) Comment on attachment 433792 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433792&action=review > Source/WebKit/UIProcess/API/wpe/WPEView.cpp:260 > +#if ENABLE(FULLSCREEN_API) It would be good to make guards check also WPE_CHECK_VERSION(1, 11, 1) to make it possible to build WebKit using older libwpe versions. > Source/WebKit/UIProcess/API/wpe/WPEView.cpp:291 > + wpe_view_backend_set_dom_fullscreen_client(m_backend, &s_fullscreenClient, this); You will need to adapt this to the function name changes in the version of the libwpe PR that we got landed. Also I expect that implementing this will make a few more layout tests pass. It would be good to look into enabling them, if so. We might need some work in WKTR (Tools/WebKitTestRunner/wpe/) to provide a fullscreening handler there and also it would be great to make it work in the MiniBrowser (Tools/MiniBrowser/wpe/). Created attachment 434998 [details]
Patch
Update to API function name changes. Add WPE version checks
(In reply to Adrian Perez from comment #4) > > You will need to adapt this to the function name changes in the version > of the libwpe PR that we got landed. The SDK with libwpe 1.11.1 has just landed in the repo. Created attachment 435062 [details]
Patch
Implement fullscreen in the minibrowser. Reset state if WPEView->setFullscreen fails
(In reply to Adrian Perez from comment #5) > Also I expect that implementing this will make a few more layout > tests pass. It would be good to look into enabling them, if so. > We might need some work in WKTR (Tools/WebKitTestRunner/wpe/) to > provide a fullscreening handler there and also it would be great > to make it work in the MiniBrowser (Tools/MiniBrowser/wpe/). I have looked into the WKTR tests and confirmed that they are not using the platform PageClientImpl->enterFullScreen path (which calls WPEView->setFullscreen). Instead the WKTR is using fullscreen code from InjectedBundlePage which sets DOM fullscreen state without calling platform fullscreen handlers. I understant that WKTR uses headless testing so it probably doesn't make sense to use real fullscreening here. Adding a WPE fullscreen handler in WKTR would not have any effect. The fullscreen related tests have the same results as before the pathc. (In reply to Lauro Moura from comment #7) > (In reply to Adrian Perez from comment #4) > > > > You will need to adapt this to the function name changes in the version > > of the libwpe PR that we got landed. > > The SDK with libwpe 1.11.1 has just landed in the repo. Great, I already tested with it instead of the local flatpak-sdk build :) (In reply to Imanol Fernandez from comment #9) > (In reply to Adrian Perez from comment #5) > > Also I expect that implementing this will make a few more layout > > tests pass. It would be good to look into enabling them, if so. > > We might need some work in WKTR (Tools/WebKitTestRunner/wpe/) to > > provide a fullscreening handler there and also it would be great > > to make it work in the MiniBrowser (Tools/MiniBrowser/wpe/). > > I have looked into the WKTR tests and confirmed that they are not using the > platform PageClientImpl->enterFullScreen path (which calls > WPEView->setFullscreen). Instead the WKTR is using fullscreen code from > InjectedBundlePage which sets DOM fullscreen state without calling platform > fullscreen handlers. I understant that WKTR uses headless testing so it > probably doesn't make sense to use real fullscreening here. Adding a WPE > fullscreen handler in WKTR would not have any effect. The fullscreen related > tests have the same results as before the pathc. > > (In reply to Lauro Moura from comment #7) > > (In reply to Adrian Perez from comment #4) > > > > > > You will need to adapt this to the function name changes in the version > > > of the libwpe PR that we got landed. > > > > The SDK with libwpe 1.11.1 has just landed in the repo. > > Great, I already tested with it instead of the local flatpak-sdk build :) And the EWS passed, too! Comment on attachment 435062 [details]
Patch
The Mac EWS failures are unrelated, so let's land this ^_^
Committed r280774 (240358@main): <https://commits.webkit.org/240358@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435062 [details]. |