Bug 44594 - [Qt] Add Visibility notification NPAPI for plugins
Summary: [Qt] Add Visibility notification NPAPI for plugins
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Girish Ramakrishnan
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2010-08-25 01:57 PDT by Girish Ramakrishnan
Modified: 2010-08-27 16:15 PDT (History)
7 users (show)

See Also:


Attachments
Set clipRect correctly on X11 (3.37 KB, patch)
2010-08-25 11:48 PDT, Girish Ramakrishnan
kenneth: review+
Details | Formatted Diff | Diff
Set clipRect correctly on X11 (3.22 KB, patch)
2010-08-26 04:44 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Set clipRect correctly on X11 (3.21 KB, patch)
2010-08-26 04:46 PDT, Girish Ramakrishnan
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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:
https://mail.mozilla.org/pipermail/plugin-futures/2010-July/000123.html

Wiki entry:
https://wiki.mozilla.org/NPAPI:VisibilityNotification

The bugzilla:
https://bugzilla.mozilla.org/show_bug.cgi?id=583109

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

WebCore/plugins/qt/PluginViewQt.cpp:588
 +          // 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