Bug 131487 - [GTK] Windowed plugins visibility doesn't work
Summary: [GTK] Windowed plugins visibility doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2014-04-10 01:59 PDT by Carlos Garcia Campos
Modified: 2014-06-25 03:39 PDT (History)
6 users (show)

See Also:


Attachments
Test case for hidden plugin (366 bytes, text/html)
2014-04-10 02:00 PDT, Carlos Garcia Campos
no flags Details
Test case for plugin inside an iframe hidden by its parent (517 bytes, application/octet-stream)
2014-04-10 02:04 PDT, Carlos Garcia Campos
no flags Details
Patch (19.20 KB, patch)
2014-04-10 02:19 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Try to fix EFL and mac builds (20.90 KB, patch)
2014-04-11 04:01 PDT, Carlos Garcia Campos
andersca: review+
Details | Formatted Diff | Diff
Patch for landing (21.06 KB, patch)
2014-06-25 00:31 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2014-04-10 01:59:41 PDT
Windowed plugins are not hidden when the element has visibility: hidden style or when the parent element is hidden (setting display: none for example). It works for windowless plugins, but for windowed plugins we need to notify the UI process about the visibility changes to show/hide the plugin widget (GtkSocket).
Comment 1 Carlos Garcia Campos 2014-04-10 02:00:52 PDT
Created attachment 229034 [details]
Test case for hidden plugin

This is a test case for visibility: hidden style. It can be tested with the totem plugin installed.
Comment 2 Carlos Garcia Campos 2014-04-10 02:04:44 PDT
Created attachment 229035 [details]
Test case for plugin inside an iframe hidden by its parent

In this case we have an iframe inside a container div. Hiding the container makes the iframe disappear, but the windowed plugin remains visible.
Comment 3 Carlos Garcia Campos 2014-04-10 02:19:52 PDT
Created attachment 229036 [details]
Patch

Implements plugins visibility change to notify the UI process in case of windowed plugins.
Comment 4 Carlos Garcia Campos 2014-04-11 04:01:53 PDT
Created attachment 229124 [details]
Try to fix EFL and mac builds
Comment 5 Anders Carlsson 2014-06-23 09:37:40 PDT
Comment on attachment 229124 [details]
Try to fix EFL and mac builds

View in context: https://bugs.webkit.org/attachment.cgi?id=229124&action=review

> Source/WebKit2/PluginProcess/PluginControllerProxy.cpp:428
> +void PluginControllerProxy::visibilityDidChange(bool visible)

Please name the parameter isVisible.

> Source/WebKit2/PluginProcess/PluginControllerProxy.cpp:619
> +void PluginControllerProxy::windowedPluginVisibilityDidChange(bool visible, uint64_t windowID)

Please name the parameter isVisible, and I think it makes more sense to put the windowID first.

> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1021
> +    if (isParentVisible() == visible)
> +        return;

I think you still want to call the Widget implementation.
Comment 6 Carlos Garcia Campos 2014-06-25 00:31:05 PDT
(In reply to comment #5)
> (From update of attachment 229124 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229124&action=review

Thanks!

> > Source/WebKit2/PluginProcess/PluginControllerProxy.cpp:428
> > +void PluginControllerProxy::visibilityDidChange(bool visible)
> 
> Please name the parameter isVisible.

Sure, renamed everywhere.

> > Source/WebKit2/PluginProcess/PluginControllerProxy.cpp:619
> > +void PluginControllerProxy::windowedPluginVisibilityDidChange(bool visible, uint64_t windowID)
> 
> Please name the parameter isVisible, and I think it makes more sense to put the windowID first.

Yes, I agree, but in windowedPluginGeometryDidChange() the window id is also the last parameter, so I'll leave consistent for now.

> > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1021
> > +    if (isParentVisible() == visible)
> > +        return;
> 
> I think you still want to call the Widget implementation.

Widget::setParentVisible() it's just m_parentVisible = visible, I'm using the same approach as ScrollView
Comment 7 Carlos Garcia Campos 2014-06-25 00:31:37 PDT
Created attachment 233795 [details]
Patch for landing
Comment 8 Carlos Garcia Campos 2014-06-25 03:39:11 PDT
Committed r170423: <http://trac.webkit.org/changeset/170423>