Bug 44594

Summary: [Qt] Add Visibility notification NPAPI for plugins
Product: WebKit Reporter: Girish Ramakrishnan <girish>
Component: Plug-insAssignee: Girish Ramakrishnan <girish>
Severity: Normal CC: abarth, ademar, christian.webkit, eric, hausmann, kenneth, webkit.review.bot
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Other   
Description Flags
Set clipRect correctly on X11
kenneth: review+
Set clipRect correctly on X11
Set clipRect correctly on X11 hausmann: review+

Description Girish Ramakrishnan 2010-08-25 01:57:07 PDT
There is some discussion on plugin-features to implement a plugin visibility notification to NPAPI. The cases are
- Plug-in instance goes out of sight after page scrolling.
- Plug-in instance comes in sight after page scrolling.
- Plug-in instance goes out of sight after browser window goes background.
- Plug-in instance comes in sight after browser window goes foreground.

See thread about it at:

Wiki entry:

The bugzilla:

This is probably interesting to all platforms but note that the public flash player does not support it and hence this task is Qt/Maemo5 specific for now.
Comment 1 Girish Ramakrishnan 2010-08-25 02:52:27 PDT
The clipRect parameter of SetWindow is used different on Mac and Linux/Windows.

On Mac, it is used to indicate the position of the plugin. When clipRect is set to 0, it means that it's not visible anymore.

On Linux/Windows, clipRect indicates the clipping rect for the painting operation and is called before every paint(). In the plugin-features thread, Adobe person mentions that clipRect is used only on Mac (https://mail.mozilla.org/pipermail/plugin-futures/2010-July/000125.html). So, I guess Flash doesn't use this value.

Currently, in Qt/WebKit, clipRect is always set to 0 on X11 in Windowless mode. In Windowed mode, we call it with the visibile rect but with a quirk (PluginQuirkDontCallSetWindowMoreThanOnce) wherein the height/width is never changed in the lifetime of the plugin.
Comment 2 Girish Ramakrishnan 2010-08-25 11:48:03 PDT
Created attachment 65453 [details]
Set clipRect correctly on X11
Comment 3 Kenneth Rohde Christiansen 2010-08-25 11:52:17 PDT
Comment on attachment 65453 [details]
Set clipRect correctly on X11

 +          // position and clipRect is relative to the Drawable
comments start with capital and ends with . Why is Drawable with a capital d?

The comments could be a bit more explanatory.
Comment 4 Girish Ramakrishnan 2010-08-26 04:44:49 PDT
Created attachment 65545 [details]
Set clipRect correctly on X11

Ok, I have a better patch now that also takes care of setting the clip rect to 0 when the plugin is scrolled out in both windowed and windowless mode.
Comment 5 WebKit Review Bot 2010-08-26 04:46:31 PDT
Attachment 65545 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/plugins/qt/PluginViewQt.cpp:588:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 2 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Girish Ramakrishnan 2010-08-26 04:46:32 PDT
Created attachment 65546 [details]
Set clipRect correctly on X11

Fix codings style
Comment 7 Girish Ramakrishnan 2010-08-26 04:57:00 PDT
Landed in r66095
Comment 8 WebKit Review Bot 2010-08-26 05:24:23 PDT
http://trac.webkit.org/changeset/66095 might have broken Qt Linux Release
Comment 9 Simon Hausmann 2010-08-26 05:45:58 PDT
Committed r66097: <http://trac.webkit.org/changeset/66097>
Comment 10 Simon Hausmann 2010-08-26 05:48:03 PDT
Committed r66095: <http://trac.webkit.org/changeset/66095>
Comment 11 Ademar Reis 2010-08-27 06:36:31 PDT
Revision r66095 cherry-picked into qtwebkit-2.1 with commit 4a3c8a09303316623216e38f5dea9e0b3f215a26
Revision r66097 cherry-picked into qtwebkit-2.1 with commit fb2dbc367407000b4a1e7d741e740d8b1417946d