| Summary: | Enforce focus check for getUserMedia | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||||||||||||
| Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||
| Severity: | Normal | CC: | berto, cdumez, cgarcia, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gustavo, hta, jer.noble, kangil.han, mifenton, philipj, pnormand, sergio, tommyw, webkit-bug-importer, youennf | ||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||
|
Description
youenn fablet
2022-01-10 04:18:50 PST
Created attachment 448740 [details]
Patch
Created attachment 448747 [details]
Patch
Comment on attachment 448747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448747&action=review > Source/WebCore/ChangeLog:10 > + The spec currently defines that we should wait for the current document to have focus but this is about to be changed and is not aligned with other browser implementations. Nit: this line should be wrapped to make it easier to read Created attachment 448757 [details]
Patch
Created attachment 448853 [details]
Patch
Created attachment 448915 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API @pnormand, can you look at GTK API test failures? I tried to fix some of them but one should remain as we are now checking that the page is focused when getDisplayMedia is called, which does not seem to be the case when running the API test in GTK. You can also probably fix the other GTK tests in the same way without introducing the private API as well. Created attachment 448929 [details]
Patch
/webkit/WebKitWebView/display-usermedia-permission-request remains broken given the page does not have focus when calling getDisplayMedia. Created attachment 448940 [details]
Patch
Created attachment 449028 [details]
Patch
(In reply to youenn fablet from comment #8) > @pnormand, can you look at GTK API test failures? > I tried to fix some of them but one should remain as we are now checking > that the page is focused when getDisplayMedia is called, which does not seem > to be the case when running the API test in GTK. > You can also probably fix the other GTK tests in the same way without > introducing the private API as well. GTK failure is tricky. The view is always focused because we are synthesizing a click event that grab the focus in the view, but under Xvfb gtk_widget_grab_focus does nothing, because there's no toplevel focus support. So, we need to pretend the view is always focused and under the active window when running under Xvfb. I'll submit an updated patch. Created attachment 449055 [details]
Patch
Doing some more testing, it does not seem like document focus is the right approach. If I call getUserMedia, the prompt will get the focus. If shortly after calling getUserMedia, I call getDisplayMedia, the call will now fail while it would have before this patch. Created attachment 449157 [details]
Patch
Created attachment 449165 [details]
Patch
Committed r288087 (246101@main): <https://commits.webkit.org/246101@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449165 [details]. |