Remove ScriptController::setAllowPopupsFromPlugin
Created attachment 97254 [details] Patch
Comment on attachment 97254 [details] Patch Attachment 97254 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8837895
Created attachment 97262 [details] Patch
Comment on attachment 97262 [details] Patch Attachment 97262 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8837913
Comment on attachment 97262 [details] Patch Attachment 97262 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/8835929
+ This API is just a poor man's UserGestureIndicator. We should use the + real deal. That's not necessarily accurate. The reason why this code was added was a specific issue with full screen plug-ins. Please talk to Anders about it.
> That's not necessarily accurate. The reason why this code was added was a specific issue with full screen plug-ins. Please talk to Anders about it. Ok, but I suspect that this code only exists because the modern way of handling user gestures didn't yet exist.
Created attachment 97363 [details] Patch
Comment on attachment 97363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97363&action=review OK. > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:786 > + UserGestureIndicator gestureIndicator(request->allowPopups() ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture); > + result = frame->script()->executeScript(jsString); Better to keep passing the bool here for now, and do all of these in a separate patch.
Alexey asked me to test whether this patch had a bad interaction with fullscreen videos on Hulu, but I wasn't able to test that because WebKit TOT doesn't seem to work correctly with the publicly released version of Safari when viewing Hulu videos. I'm pretty sure this patch is ok, but someone internal to Apple might want to double-check with whatever internal builds of Safari exist that might work properly.
Committed r89001: <http://trac.webkit.org/changeset/89001>
(In reply to comment #10) > Alexey asked me to test whether this patch had a bad interaction with fullscreen videos on Hulu, but I wasn't able to test that because WebKit TOT doesn't seem to work correctly with the publicly released version of Safari when viewing Hulu videos. What didn't work for you? Safari 5.0.5 + nightly r88920 seem to work perfectly fine when viewing Hulu videos full screen.
> What didn't work for you? Safari 5.0.5 + nightly r88920 seem to work perfectly fine when viewing Hulu videos full screen. Noel tells me I need to build the 32 bit version of WebKit, which I'm doing now.
> What didn't work for you? Safari 5.0.5 + nightly r88920 seem to work perfectly fine when viewing Hulu videos full screen. The symptoms are that Safari beachballs waiting for some sort of mach channel. I can reproduce and get the stack if you like, but I'm halfway through this 32 bit build at the moment.
> Alexey asked me to test whether this patch had a bad interaction with fullscreen videos on Hulu Seem to work fine. The popup is created as expected. The full-screen video stays open instead of closing, but that seems unrelated to this bug.
(In reply to comment #15) > > Alexey asked me to test whether this patch had a bad interaction with fullscreen videos on Hulu > > Seem to work fine. The popup is created as expected. The full-screen video stays open instead of closing, but that seems unrelated to this bug. Confirmed. The behavior is the same in the WebKit nightly built before this patch landed.
Cool, thanks for testing. And Firefox has the same behavior.