WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
14750
[gtk] Implement plugin support in GTK backend
https://bugs.webkit.org/show_bug.cgi?id=14750
Summary
[gtk] Implement plugin support in GTK backend
Bastien Nocera
Reported
2007-07-24 10:27:35 PDT
The GTK+ backend currently doesn't implement any plug-ins support. It should do so probably supporting the old NPAPI with XEmbed and GTK2 support, as well as using the more native API used by other ports.
Attachments
Start at plug-ins patch
(123.46 KB, patch)
2007-11-02 14:41 PDT
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Update plug-ins patch
(171.25 KB, patch)
2007-11-13 13:40 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch for the Shared type removal
(171.31 KB, patch)
2007-11-14 10:55 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch to handle XEmbed and Xt plug-ins both
(172.63 KB, patch)
2007-11-15 09:45 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated to build against trunk
(177.53 KB, patch)
2007-12-11 10:44 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch for KURL class changes and without other patches included
(165.66 KB, patch)
2007-12-12 14:01 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated Giant Patch With Everything
(177.33 KB, patch)
2007-12-13 13:56 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated giant patch for trunk
(174.48 KB, patch)
2007-12-17 07:58 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch for trunk today
(174.02 KB, patch)
2007-12-18 09:16 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
NPAPI scriptability test
(64.23 KB, application/x-tar)
2007-12-18 09:26 PST
,
Kai Hendry
no flags
Details
Update patch against r28911
(166.93 KB, patch)
2007-12-20 15:07 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated plug-ins patch
(166.90 KB, patch)
2007-12-24 13:02 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch to fix scrolling too
(165.94 KB, patch)
2007-12-24 14:46 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Update to fix a Gtk-CRITICAL when destroying the widget
(165.96 KB, patch)
2007-12-25 09:16 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch to build with autotools also, against r29084
(167.96 KB, patch)
2008-01-02 09:13 PST
,
Rodney Dawes
mrowe
: review-
Details
Formatted Diff
Diff
Updated patch against r29305
(167.97 KB, patch)
2008-01-08 11:55 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r29338
(163.99 KB, patch)
2008-01-09 16:14 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r29531
(1.00 KB, patch)
2008-01-16 09:43 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r29725
(134.55 KB, patch)
2008-01-22 15:48 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r29742
(134.35 KB, patch)
2008-01-23 12:42 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r29792
(134.03 KB, patch)
2008-01-25 09:24 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r29914
(133.10 KB, patch)
2008-02-01 10:10 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r29975
(136.53 KB, patch)
2008-02-04 14:05 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r30005
(133.83 KB, patch)
2008-02-05 10:51 PST
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
64-bit fixes
(2.12 KB, patch)
2008-02-25 03:51 PST
,
Gwenole Beauchesne
no flags
Details
Formatted Diff
Diff
Updated patch against r31217
(93.30 KB, patch)
2008-03-21 13:58 PDT
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch
(94.44 KB, patch)
2008-03-24 11:59 PDT
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch against r31326
(95.37 KB, patch)
2008-03-26 13:48 PDT
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
alternative updated version of patch (against 31893, despite the filename)
(93.22 KB, patch)
2008-04-16 15:15 PDT
,
Adam Williamson
no flags
Details
Formatted Diff
Diff
patch for Plugin updated to r31990
(25.58 KB, patch)
2008-04-17 01:22 PDT
,
Sriram Neelakandan
no flags
Details
Formatted Diff
Diff
Updated patch against r31990 - fixed missing files
(94.89 KB, patch)
2008-04-17 06:00 PDT
,
Sriram Neelakandan
no flags
Details
Formatted Diff
Diff
fixes sri's latest patch: this one doesn't break the Qt build
(95.79 KB, patch)
2008-04-17 14:48 PDT
,
Adam Williamson
no flags
Details
Formatted Diff
Diff
Adds NPAPI plugin support for Gtk and Qt-x11.
(123.05 KB, patch)
2008-04-23 03:28 PDT
,
marcoil
no flags
Details
Formatted Diff
Diff
Adds NPAPI plugin support for Gtk and Qt-x11, r32489
(123.88 KB, patch)
2008-04-24 09:01 PDT
,
marcoil
no flags
Details
Formatted Diff
Diff
64-bit fixes
(1.13 KB, patch)
2008-04-29 04:42 PDT
,
Gwenole Beauchesne
no flags
Details
Formatted Diff
Diff
Specialize NPN_GetValue()
(2.32 KB, patch)
2008-04-29 04:45 PDT
,
Gwenole Beauchesne
no flags
Details
Formatted Diff
Diff
Adds NPAPI plugin support for Gtk and Qt-x11, r32726
(127.02 KB, patch)
2008-04-30 04:28 PDT
,
marcoil
no flags
Details
Formatted Diff
Diff
Show Obsolete
(32)
View All
Add attachment
proposed patch, testcase, etc.
Kai Hendry
Comment 1
2007-10-31 08:02:43 PDT
Jean-Charles VERDIE from Pleyo (they are doing some mobile Webkit port) mentioned that he might be working on plugin support. I wonder if there is any overlap.
Jean-Charles Verdié
Comment 2
2007-10-31 08:15:46 PDT
To be clearer: We are trying to understand what has been done on the windows port, split the common part and the specific part, then to understand how it works and give this code back to webkit to make it possible to implement using different backend while we won't do more than re-implementing Windows (for testing purposes) and our own "owBal" implementation (ours at sand-labs.org)
Rodney Dawes
Comment 3
2007-10-31 14:07:25 PDT
(In reply to
comment #2
)
> To be clearer: We are trying to understand what has been done on the windows > port, split the common part and the specific part, then to understand how it > works and give this code back to webkit to make it possible to implement using > different backend while we won't do more than re-implementing Windows (for > testing purposes) and our own "owBal" implementation (ours at sand-labs.org)
Is this plug-in code viewable somewhere?
Rodney Dawes
Comment 4
2007-11-02 14:41:22 PDT
Created
attachment 16998
[details]
Start at plug-ins patch Here is my patch which mostly implements everything needed for plug-ins to work. It's got some debug printfs() lying around, and the code isn't perfectly matching the style guide yet. If flash is found, it will crash the browser on pages that use it. Flash requires use of X toolkit bits still, and without these, it simply just crashes whatever loads it. Implementing this requires a custom widget to integrate the necessary bits with GTK+. Unfortunately, the GtkSocket I'm trying to use isn't getting sized properly or rendered on screen. It seems to get parented, mapped, and the plug-in seems to get embedded. But, the size never gets allocated to anything > 1x1, with a position of -1,-1. I need to get this widget to embed and render properly before I can really do anything else, unfortunately. Feel free to try the patch though. It would be great if someone could figure out why it's not working right. I still don't understand exactly how the WebKitPage parenting and stuff is supposed to work.
Mark Rowe (bdash)
Comment 5
2007-11-05 13:22:58 PST
Alp helped Rodney work out the issue with the widget sizing/positioning, and I think he's back underway on his plugin patch.
Rodney Dawes
Comment 6
2007-11-13 13:40:15 PST
Created
attachment 17239
[details]
Update plug-ins patch Here is an updated version of the plug-ins patch. This version only supports Xt windowed plug-ins. Next version of the patch will support both. Adobe Flash 9 works with this version. You can watch the videos on
http://homestarrunner.com/
. Youtube and similar flash video player sites will load the flash, but only stream the first N bytes of the actual video to the player. This is because the PluginStreamGtk code is not fully complete. Hopefully I should have it fixed in the next version also. I'm also having trouble getting the Xt plug-ins to scroll correctly. It doesn't seem to update the position properly. But all in all it's nice to see flash working. :)
Rodney Dawes
Comment 7
2007-11-14 10:55:35 PST
Created
attachment 17273
[details]
Updated patch for the Shared type removal
Nigel Tao
Comment 8
2007-11-14 15:24:34 PST
Just with the patch, please replace tabs by spaces.
http://webkit.org/coding/coding-style.html
Rodney Dawes
Comment 9
2007-11-15 09:45:41 PST
Created
attachment 17297
[details]
Updated patch to handle XEmbed and Xt plug-ins both Ok. Here is an updated version of the patch. It handles both XEmbed and Xt plug-ins the same way gecko does, by checking the value of NPPVneedsXEmbed with the plug-in's GetValue implementation. I'd like to start splitting up the patch logically now and getting some pieces in. Perhaps the PluginDatabaseGtk impl and the ability to build with XP_UNIX enabled (but only when building for X11). Are there standard locations for plug-ins on win32 and osx that we should be checking? I'd like to get that support in PluginDatabaseGtk also.
Kai Hendry
Comment 10
2007-11-27 15:25:10 PST
Built and tested dobey's "Updated patch to handle XEmbed and Xt plug-ins both" patch with a
r27911
WebKit checkout. Macromedia Flash playback on YouTube worked. =) Be good to start merging this patch in. I hope to help dobey to improve the NPAPI implementation with some scriptability tests.
Rodney Dawes
Comment 11
2007-12-11 10:44:03 PST
Created
attachment 17851
[details]
Updated to build against trunk
Rodney Dawes
Comment 12
2007-12-12 14:01:35 PST
Created
attachment 17869
[details]
Updated patch for KURL class changes and without other patches included This is an updated version of the patch for the changes to the KURL class. This version of the patch also does not include changes from other patches, as previous versions did. You will need the patches in the bugs I'm adding as dependencies as well for a full experience.
Rodney Dawes
Comment 13
2007-12-12 14:03:36 PST
I can't change the DependsOn field it seems, so you will need the patches from bugs #16389 #15669 and #16071 as well.
Rodney Dawes
Comment 14
2007-12-13 13:56:33 PST
Created
attachment 17880
[details]
Updated Giant Patch With Everything There are still some conflicts between patches, so here is another huge patch with all the changes needed.
Rodney Dawes
Comment 15
2007-12-17 07:58:30 PST
Created
attachment 17963
[details]
Updated giant patch for trunk
Rodney Dawes
Comment 16
2007-12-18 09:16:21 PST
Created
attachment 17973
[details]
Updated patch for trunk today
Kai Hendry
Comment 17
2007-12-18 09:26:32 PST
Created
attachment 17974
[details]
NPAPI scriptability test
http://wiki.webvm.net/
is the site where the Aplix Corporation is co-ordinating their NPAPI plugin efforts.
Rodney Dawes
Comment 18
2007-12-20 15:07:15 PST
Created
attachment 18017
[details]
Update patch against
r28911
Rodney Dawes
Comment 19
2007-12-24 13:02:20 PST
Created
attachment 18094
[details]
Updated plug-ins patch This is just a quick update to fix some warnings about variable initialization ordering in plugins/gtk/ and use of char* instead of const char* for string constants in PluginDebug.h.
Rodney Dawes
Comment 20
2007-12-24 14:46:24 PST
Created
attachment 18096
[details]
Updated patch to fix scrolling too
Rodney Dawes
Comment 21
2007-12-25 09:16:35 PST
Created
attachment 18108
[details]
Update to fix a Gtk-CRITICAL when destroying the widget
Rodney Dawes
Comment 22
2008-01-02 09:13:48 PST
Created
attachment 18239
[details]
Updated patch to build with autotools also, against
r29084
Mark Rowe (bdash)
Comment 23
2008-01-02 09:45:26 PST
Comment on
attachment 18239
[details]
Updated patch to build with autotools also, against
r29084
Sorry that this has been relatively untouched for so long. There are a few relatively serious issues here: PluginViewGtk has a large amount of duplicated code with PluginViewWin, some of which could trivially be factored out (handlePost, parseRFC822HeaderFields and friends). It also contains many chunks of commented-out code, unused variables (kWebPluginViewWindowClassName, kWebPluginViewProperty) and plugin quirks which are probably not needed on non-Windows platforms. I'm not sure that PluginMessageThrottlerGtk would be needed on non-Windows platforms as I believe the Windows equivalent exists due to peculiarities with paint events and the Windows message queue, but I suspect Anders would know more about that. I suspect the references to XID might need to be conditional on X11 being available, both in PluginViewGtk.cpp and PluginViewGtk.h. PluginViewGtk seems to contain a bunch of references to "wndProc" still which is a Windows concept that seems out of place in this Gtk code. PluginStreamGtk seems to have a large amount of duplicate code with PluginStreamWin, only the Gtk version looks to be broken in the case of NP_ASFILE/NP_ASFILEONLY streams. It should be possible to share the majority of this code with Windows, and eventually share with the Mac when that code is refactored. With a bit more polish we should be able to get this landed soon.
Rodney Dawes
Comment 24
2008-01-08 11:55:38 PST
Created
attachment 18331
[details]
Updated patch against
r29305
Rodney Dawes
Comment 25
2008-01-09 16:14:52 PST
Created
attachment 18356
[details]
Updated patch against
r29338
This patch also resolves several of the issues mentioned in Mark's quick review. Curl also seems to be crashing quite a bit for me now as well. I'm not sure why.
Michael Emmel
Comment 26
2008-01-09 18:08:06 PST
(In reply to
comment #25
)
> Created an attachment (id=18356) [edit] > Updated patch against
r29338
> > This patch also resolves several of the issues mentioned in Mark's quick > review. Curl also seems to be crashing quite a bit for me now as well. I'm not > sure why. >
If your using files instead of http the version in the head probably will crash. Its got quite a few nasty bugs that are not obvious. I've got another version that uses the latest curl with file handles handed to the OS for select works much better than polling.
Rodney Dawes
Comment 27
2008-01-09 18:45:51 PST
(In reply to
comment #26
)
> If your using files instead of http the version in the head probably will > crash. > Its got quite a few nasty bugs that are not obvious. > I've got another version that uses the latest curl with file handles handed to > the OS for select works much better than polling.
No, it's crashing when trying to stream data for flash. Most notably it occurs with
http://burststudio.com/kitten.html
after clicking "start game" or when trying to seek past the buffered data in a flash video player, such as on youtube.
Michael Emmel
Comment 28
2008-01-09 19:15:25 PST
(In reply to
comment #27
)
> (In reply to
comment #26
) > > If your using files instead of http the version in the head probably will > > crash. > > Its got quite a few nasty bugs that are not obvious. > > I've got another version that uses the latest curl with file handles handed to > > the OS for select works much better than polling. > > No, it's crashing when trying to stream data for flash. Most notably it occurs > with
http://burststudio.com/kitten.html
after clicking "start game" or when > trying to seek past the buffered data in a flash video player, such as on > youtube. >
Yeah I could see streaming being a problem also. Anything but a reasonably a http connection that closes fast enough will probably crash. I'm meeting with Alp Toker in a few weeks and I'll get a patch out soon after. Right now I've got to get a demo going. The new version is doing stuff like this. curl_multi_setopt(d->m_curlMultiHandle, CURLMOPT_SOCKETFUNCTION, socketCallback); curl_multi_setopt(d->m_curlMultiHandle, CURLMOPT_SOCKETDATA,this); curl_multi_setopt(d->m_curlMultiHandle, CURLMOPT_TIMERFUNCTION,timerCallback); curl_multi_setopt(d->m_curlMultiHandle, CURLMOPT_TIMERDATA,this); Which is a lot better and has not crashed yet for me. Curl still has a few bogus timer callbacks but this can be fixed a bit later. The big issue left is msg's to the multi handle should be possible from a callback. But I'll need to send a patch in for curl itself to fix this.
Rodney Dawes
Comment 29
2008-01-16 09:43:55 PST
Created
attachment 18473
[details]
Updated patch against
r29531
Rodney Dawes
Comment 30
2008-01-22 15:48:00 PST
Created
attachment 18606
[details]
Updated patch against
r29725
Alp Toker
Comment 31
2008-01-23 04:03:04 PST
(In reply to
comment #30
)
> Created an attachment (id=18606) [edit] > Updated patch against
r29725
>
Could you add a comment about gtk2xtbin.c and any other imported sources in the ChangeLog entry? We should try to keep track of all "bundled" source code from other projects so we can keep up to date with their bugfixes, and maybe even send back our fixes to the code. If the intention is not to bundle the code but to adopt it, then it needs to be modified to follow WebKit coding style. Bundled code should keep upstream coding style -- i think in this case it's better to treat the code as bundled to avoid needless changes. What do you think?
Rodney Dawes
Comment 32
2008-01-23 12:42:57 PST
Created
attachment 18623
[details]
Updated patch against
r29742
Rodney Dawes
Comment 33
2008-01-25 09:24:02 PST
Created
attachment 18691
[details]
Updated patch against
r29792
This is an updated patch to deal with the new PluginQuirkSet class introduced in recent builds.
Rodney Dawes
Comment 34
2008-01-25 09:55:23 PST
> Could you add a comment about gtk2xtbin.c and any other imported sources in the > ChangeLog entry?
That's the plan, yes.
Rodney Dawes
Comment 35
2008-02-01 10:10:47 PST
Created
attachment 18849
[details]
Updated patch against
r29914
Alp Toker
Comment 36
2008-02-03 23:39:03 PST
(In reply to
comment #35
)
> Created an attachment (id=18849) [edit] > Updated patch against
r29914
>
Is this patch intended for review? It won't be looked at unless it's set r? and has ChangeLog entries. I noticed a few tabs where they shouldn't be. I'm still interested in finding out what the origin of the bundled code is. Having some kind of trail is important.
Rodney Dawes
Comment 37
2008-02-04 09:02:14 PST
(In reply to
comment #36
)
> Is this patch intended for review? It won't be looked at unless it's set r? and > has ChangeLog entries.
Not entirely yet. Due to the need to refactor the code, this patch will have to be updated when the refactoring of the win32 plug-in code to be shared, to use the shared implementation for most things.
> I noticed a few tabs where they shouldn't be. I'm still interested in finding > out what the origin of the bundled code is. Having some kind of trail is > important.
The "original author" is in the code's license block, and as I said before, in
comment 34
, the origin of the code will also be noted in the ChangeLog when it gets written (after all the refactoring work).
Rodney Dawes
Comment 38
2008-02-04 14:05:14 PST
Created
attachment 18918
[details]
Updated patch against
r29975
Rodney Dawes
Comment 39
2008-02-05 10:51:02 PST
Created
attachment 18936
[details]
Updated patch against
r30005
Gwenole Beauchesne
Comment 40
2008-02-25 03:51:44 PST
Created
attachment 19342
[details]
64-bit fixes This is a patch to fix 64-bit issues brought by the NPAPI/gtk giant patch. I have only tested with Flash 9.0.115 and webkit safari-3.1-branch.
Rodney Dawes
Comment 41
2008-03-21 13:58:48 PDT
Created
attachment 19936
[details]
Updated patch against
r31217
Here's an updated patch against
r31217
which works with the refactored plugin api code.
Jasper Bryant-Greene
Comment 42
2008-03-24 05:47:19 PDT
I think the MPL licensing needs to be deleted in that patch so that the GPL applies instead? If it's now ready for review then please add a ChangeLog entry and mark it r?
Mark Rowe (bdash)
Comment 43
2008-03-24 09:54:29 PDT
(In reply to
comment #42
)
> I think the MPL licensing needs to be deleted in that patch so that the GPL > applies instead?
The GPL is definitely not acceptable (Perhaps you meant LGPL?). I think it may be preferable to leave the original license header on the code to indicate its origin and the license under which we have adopted it. I'm not an expert on these sorts of things though.
Rodney Dawes
Comment 44
2008-03-24 11:59:04 PDT
Created
attachment 20011
[details]
Updated patch
Jasper Bryant-Greene
Comment 45
2008-03-24 13:12:20 PDT
(In reply to
comment #43
)
> The GPL is definitely not acceptable (Perhaps you meant LGPL?). I think it may > be preferable to leave the original license header on the code to indicate its > origin and the license under which we have adopted it. I'm not an expert on > these sorts of things though.
Sorry, I did mean LGPL. The reason I say that the MPL should be deleted is that the MPL specifically says this: * Alternatively, the contents of this file may be used under the terms of * either the GNU General Public License Version 2 or later (the "GPL"), or * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), * in which case the provisions of the GPL or the LGPL are applicable instead * of those above. If you wish to allow use of your version of this file only * under the terms of either the GPL or the LGPL, and not to allow others to * use your version of this file under the terms of the MPL, indicate your * decision by deleting the provisions above and replace them with the notice * and other provisions required by the GPL or the LGPL. If you do not delete * the provisions above, a recipient may use your version of this file under * the terms of any one of the MPL, the GPL or the LGPL.
Mike Hommey
Comment 46
2008-03-24 23:10:59 PDT
(In reply to
comment #45
)
> Sorry, I did mean LGPL. The reason I say that the MPL should be deleted is that > the MPL specifically says this:
(...) But why would you want that ? It would be better for everyont if you kept the original license, so that modifications made here could be taken back under the original tri-license terms.
Jasper Bryant-Greene
Comment 47
2008-03-24 23:14:45 PDT
(In reply to
comment #46
)
> (In reply to
comment #45
) > > Sorry, I did mean LGPL. The reason I say that the MPL should be deleted is that > > the MPL specifically says this: > (...) > > But why would you want that ? It would be better for everyont if you kept the > original license, so that modifications made here could be taken back under the > original tri-license terms.
The reason I think it would be necessary is that code contributed to WebKit needs to be licensed under either LGPL or new BSD. If the license gives the user the option to take it under the GPL or MPL, I would think that is incompatible. However there may have been some confusion here. Is this code merely being bundled, and not actually being contributed to form part of WebKit? If that's the case then of course there's no problem with keeping the original license.
Mike Hommey
Comment 48
2008-03-24 23:52:02 PDT
(In reply to
comment #47
)
> The reason I think it would be necessary is that code contributed to WebKit > needs to be licensed under either LGPL or new BSD. If the license gives the > user the option to take it under the GPL or MPL, I would think that is > incompatible.
There is no incompatibility with having more licensing terms for a snippet of code than for the rest of the tree, as far as licenses are compatible. And obviously, a code snippet that is licensed under the terms of MPL, GPL *and* LGPL can be linked to LGPL or BSD licensed code. (BTW, code contributed to WebKit is usually licensed under LGPL, 3-clause BSD *or* 2-clause BSD, which is actually quite annoying, because each file has a different license. Why not have *all* dual-licensed under 3-clause BSD and LGPL ?)
Rodney Dawes
Comment 49
2008-03-26 13:48:04 PDT
Created
attachment 20098
[details]
Updated patch against
r31326
Here is an updated patch which removes a couple extra lines of debugging outut, and fixes a crash when calling refresh() with an already populated PluginDatabase, exposed by
http://www.java.com/en/download/installed.jsp
.
Adam Williamson
Comment 50
2008-04-16 15:13:18 PDT
I'm trying to add this patch to the Mandriva webkit package. I had to rediff it a bit for current SVN webkit, but finally got it to compile clean. However, it doesn't seem to actually work...I'm attaching my rediffed version of the patch. (It also has the ChangeLog bits taken out as it has to apply against the snapshot tarballs for us, not against the tree). a) hope some of the rediffing is useful and b) does anyone know what might be wrong? When I try to use, say, YouTube - in Epiphany or Midori - with the patched webkitgtk, I just get the same 'no Flash or JavaScript disabled' error.
Adam Williamson
Comment 51
2008-04-16 15:15:30 PDT
Created
attachment 20604
[details]
alternative updated version of patch (against 31893, despite the filename) this is an amateur attempt at a rediff of the 31326 patch against 31893. as noted in my comment, it doesn't seem to work for me, for some reason. all ChangeLog changes have been taken out, so it applies against the snapshot tarball. if any of my changes are useful they can happily be used under any licensing terms at all.
Sriram Neelakandan
Comment 52
2008-04-17 01:22:17 PDT
Created
attachment 20617
[details]
patch for Plugin updated to
r31990
Older Plugin patch has been updated to
r31990
- Modified original patch to use Directory instead of Path - Implemented getFileModificationTime for WebCore/platform/gtk/FileSystemGtk.cpp The getFileModificationTime is necessary for plugins to load as per the new changes in the trunk.
Sriram Neelakandan
Comment 53
2008-04-17 01:27:03 PDT
Comment on
attachment 20617
[details]
patch for Plugin updated to
r31990
NOTE: the earlier patch
https://bugs.webkit.org/attachment.cgi?id=20098
has been adopted for
r31990
Adam Williamson
Comment 54
2008-04-17 03:50:34 PDT
Sriram, your updated patch still refers to PluginDataGtk.cpp in GNUmakefile.am , even though you removed that file from the patch, so it doesn't build. That line should be taken out of GNUmakefile.am , I guess.
Adam Williamson
Comment 55
2008-04-17 03:59:16 PDT
Same thing with PluginPackageGtk.cpp . Is this patch just broken? Or is it meant to be a serial patch together with 31326, in which case it's still wrong as it still contains some of the same things as 31326?
Sriram Neelakandan
Comment 56
2008-04-17 05:51:57 PDT
(In reply to
comment #55
)
> Same thing with PluginPackageGtk.cpp . > > Is this patch just broken? Or is it meant to be a serial patch together with > 31326, in which case it's still wrong as it still contains some of the same > things as 31326? >
Thanks Adam. I found the problem .. i did svn-create-patch .. and some files were not added to snv. I will fix that and upload a new patch
Sriram Neelakandan
Comment 57
2008-04-17 06:00:19 PDT
Created
attachment 20621
[details]
Updated patch against
r31990
- fixed missing files Fixed my previous patch, that had a few missing files. This one should compile fine
Adam Williamson
Comment 58
2008-04-17 14:46:43 PDT
This version works (great!), but breaks the Qt build. The 31326 patch had the same problem. I'm attaching a slightly modified version of sri's patch which fixes that issue. Tested with this patch, Midori and Epiphany-webkit can both do Youtube fine. I had to remove my Java plugin though (I use the IcedTea one), as I was getting this error: [adamw@lenovo SPECS]$ epiphany Init plugins called Module Load Failed :/usr/lib/mozilla/plugins/libjavaplugin.so, Error:libxpcom.so: cannot open shared object file: No such file or directory which caused the browser to crash (same in Midori). Not sure if this is a problem with the patch, the plugin, or just down to packaging on our (Mandriva's) side. With the Java plugin removed, Flash seems to work fine.
Adam Williamson
Comment 59
2008-04-17 14:48:41 PDT
Created
attachment 20639
[details]
fixes sri's latest patch: this one doesn't break the Qt build
Adam Williamson
Comment 60
2008-04-17 14:50:35 PDT
yeah, with either Sun or IcedTea Java plugin, Epiphany with this patch crashes on startup for me, and Midori crashes as soon as I go to any site which would require a plugin to be loaded. Anyone else seeing this? With Sun Java I don't get the libxpcom.so error, but the browser still crashes.
Mike Hommey
Comment 61
2008-04-18 08:54:44 PDT
is -lnspr4 really needed in GNUmakefile.am ?
Alp Toker
Comment 62
2008-04-20 23:15:23 PDT
I landed the xtbin bundled sources in
r32287
. I hope this makes the plugin patch smaller and easier to review.
Alp Toker
Comment 63
2008-04-20 23:28:05 PDT
The changes to .pro files should be dropped from this patch as well as the already-landed xtbin files.
Sriram Neelakandan
Comment 64
2008-04-21 22:29:15 PDT
XtBin changes landed -
http://trac.webkit.org/projects/webkit/changeset/32287
FileSystemGtk landed -
http://trac.webkit.org/projects/webkit/changeset/32288
Shen Chuan-Hsing
Comment 65
2008-04-22 17:41:37 PDT
(In reply to
comment #59
)
> Created an attachment (id=20639) [edit] > fixes sri's latest patch: this one doesn't break the Qt build >
PluginView::init() { .... if (m_isWindowed) { #if defined(GDK_WINDOWING_X11) NPSetWindowCallbackStruct ws; .... m_npWindow.ws_info = &ws; #endif } .... if (!(m_plugin->quirks().contains(PluginQuirkDeferFirstSetWindowCall))) setNPWindowRect(frameGeometry()); } void PluginView::setNPWindowRect(const IntRect& rect) { // ((NPSetWindowCallbackStruct*)m_npWindow.ws_info)->display becomes null and crash. }
marcoil
Comment 66
2008-04-23 03:28:37 PDT
Created
attachment 20769
[details]
Adds NPAPI plugin support for Gtk and Qt-x11. This patch adds NPAPI plugin support for both Gtk (XEmbed, Xt and some support for window-less plugins at the moment) and Qt-x11 (only XEmbed plugins at the moment). It's based on Rodney Dawes' previous work. It also moves some code from win to platform-independent as it's used in all plugin support code. I have tested this patch with both the diamondx plugin (works on both platforms) and Adobe Flash Player, which works but has problems with scrolling on Gtk. Some other plugins don't load because they depend on XPCOM. If anyone knows of an Xt plugin I could test with, please send it my way.
Adam Williamson
Comment 67
2008-04-23 05:35:27 PDT
for the record, marcoil's patch is against rev 32430. I will test it shortly...
Gwenole Beauchesne
Comment 68
2008-04-23 06:08:27 PDT
Hi, you can try Acrobat Reader 5 or Flash 7 plugins which use Xt IIRC.
Simon Hausmann
Comment 69
2008-04-23 06:41:33 PDT
First of all, great job!! I have a few small comments: +#elif PLATFORM(QT) && PLATFORM(UNIX) +#define ENABLE_NETSCAPE_PLUGIN_API 1 In order to not break the Qt/Mac and the Qt/Embedded build I suggest to make this even stricter by perhaps simply using #elif PLATFORM(QT) && defined(Q_WS_X11) In PluginPackageQt.cpp, PluginPackage::load() if loading the module fails I think the QLibrary should be deleted and m_module be set back to a null pointer. Also it seems that unloadModule() in FileSystemQt.cpp does not delete the QLibrary itself, it just calls unload(), which it seems the caller in PluginPackage::freeLibraryTimerFired() expects the function to free any allocated resources. In PluginViewQt.cpp in the PluginView destructor: + if (m_window) + delete m_window; I think the if() is safe to remove :) Other than that I looks good to me on the Qt side.
Alp Toker
Comment 70
2008-04-23 06:48:47 PDT
(In reply to
comment #66
)
> Created an attachment (id=20769) [edit] > Adds NPAPI plugin support for Gtk and Qt-x11. > > This patch adds NPAPI plugin support for both Gtk (XEmbed, Xt and some support > for window-less plugins at the moment) and Qt-x11 (only XEmbed plugins at the > moment). It's based on Rodney Dawes' previous work. > > It also moves some code from win to platform-independent as it's used in all > plugin support code. > > I have tested this patch with both the diamondx plugin (works on both > platforms) and Adobe Flash Player, which works but has problems with scrolling > on Gtk. Some other plugins don't load because they depend on XPCOM. If anyone > knows of an Xt plugin I could test with, please send it my way. >
Looks OK on the GTK+ side. What's -lnspr4 there for? We can't start introducing a dependency on packages and libraries from another browser. Once the Qt issues are addressed I can look into landing this.
marcoil
Comment 71
2008-04-23 08:44:56 PDT
(In reply to
comment #69
)
> First of all, great job!! >
Thanks!
> +#elif PLATFORM(QT) && PLATFORM(UNIX) > +#define ENABLE_NETSCAPE_PLUGIN_API 1 > > In order to not break the Qt/Mac and the Qt/Embedded build I suggest to make > this even stricter by perhaps simply using > > #elif PLATFORM(QT) && defined(Q_WS_X11) >
That's what I did in the first place, but it doesn't work because at that time no Qt header has been included yet, so Q_WS_X11 is not defined. Should I define ENABLE_NETSCAPE_PLUGIN_API in WebCore.pro instead? I'll make a new patch for the other comments.
Adam Williamson
Comment 72
2008-04-23 09:55:42 PDT
I can confirm that the new patch from marcoil builds clean against the latest nightly (32416). It also basically works, and having a Java plugin installed no longer causes the browser to crash. However, it seems basically flaky. Java plugin still doesn't actually work (though this may be known). Sometimes playing video at Youtube works, but quite often, Midori just crashes. I can also get a reproducible crash simply by running Acid3 (
http://acid3.acidtests.org/
)with the patch applied: it crashes around 76/100. Without the patch, the test completes fine. Have not yet tested with Ephy, I need to rebuild it.
Adam Williamson
Comment 73
2008-04-23 15:58:42 PDT
Epiphany results are basically identical to Midori. Also flaky with Flash, nothing with Java, crashes running Acid3.
Shen Chuan-Hsing
Comment 74
2008-04-23 18:55:15 PDT
WebCore/plugins/gtk/PluginViewGtk.cpp NPSetWindowCallbackStruct ws; ws.type = 0; if (m_needsXEmbed) { ws.display = GDK_WINDOW_XDISPLAY(m_window->window); ws.visual = GDK_VISUAL_XVISUAL(gdk_drawable_get_visual(GDK_DRAWABLE(m_window->window))); ws.depth = gdk_drawable_get_visual(GDK_DRAWABLE(m_window->window))->depth; ws.colormap = GDK_COLORMAP_XCOLORMAP(gdk_drawable_get_colormap(GDK_DRAWABLE(m_window->window))); } else { ws.display = GTK_XTBIN(m_window)->xtdisplay; ws.visual = GTK_XTBIN(m_window)->xtclient.xtvisual; ws.depth = GTK_XTBIN(m_window)->xtclient.xtdepth; ws.colormap = GTK_XTBIN(m_window)->xtclient.xtcolormap; XFlush (ws.display); } m_npWindow.ws_info = &ws; should be: NPSetWindowCallbackStruct *ws = new NPSetWindowCallbackStruct(); ws->type = 0; if (m_needsXEmbed) { ws->display = GDK_WINDOW_XDISPLAY(m_window->window); ws->visual = GDK_VISUAL_XVISUAL(gdk_drawable_get_visual(GDK_DRAWABLE(m_window->window))); ws->depth = gdk_drawable_get_visual(GDK_DRAWABLE(m_window->window))->depth; ws->colormap = GDK_COLORMAP_XCOLORMAP(gdk_drawable_get_colormap(GDK_DRAWABLE(m_window->window))); } else { ws->display = GTK_XTBIN(m_window)->xtdisplay; ws->visual = GTK_XTBIN(m_window)->xtclient.xtvisual; ws->depth = GTK_XTBIN(m_window)->xtclient.xtdepth; ws->colormap = GTK_XTBIN(m_window)->xtclient.xtcolormap; XFlush (ws->display); } m_npWindow.ws_info = ws;
Sriram Neelakandan
Comment 75
2008-04-23 22:57:54 PDT
> > NPSetWindowCallbackStruct ws; > > should be: > > NPSetWindowCallbackStruct *ws = new NPSetWindowCallbackStruct();
I agree we cannot alloc on stack and store in a class member.. but the new struct is a leak per plugin so we need this also : PluginView::stop() { ... .. if (m_isWindowed && m_npWindow.ws_info) delete m_npWindow.ws_info }
marcoil
Comment 76
2008-04-24 09:01:42 PDT
Created
attachment 20794
[details]
Adds NPAPI plugin support for Gtk and Qt-x11,
r32489
This new patch addresses comments made here: - ENABLE_NETSCAPE_PLUGIN_API on Qt is defined in WebCore.pro, as it was difficult to do it in Platform.h. - QtLibraries are deleted if correctly unloaded. - I removed the dependency on libnspr4, I don't know where that came from. Right now (on my setup), testing with GtkLauncher and QtLauncher: - Acid3 gets to 94/100 on both platforms without crashing, although it gets "linktest failed" on Qt, don't know if this is due to the patch. - Flash semi-works in a very flacky way. It has problems with scrolling on Gtk and YouTube videos stutter on Qt (other videos sites work, though). XEmbed implementations are different and I've sent a patch for Qt that solves this a little. - Java doesn't load and I'm currently investigating why. - Some other plugins don't load because they depend on XPCOM, i.e., totem video plugins. Others don't work on Qt because of Gtk dependencies, like Gnash. - Adobe Reader doesn't load because it needs display info before any PluginView has been created, which will be difficult to solve right now.
Adam Williamson
Comment 77
2008-04-24 09:04:31 PDT
will incorporate the patch into our build and do my own tests later. Thanks. I thought current Webkit was supposed to hit 100/100 on acid3?
marcoil
Comment 78
2008-04-24 09:56:57 PDT
(In reply to
comment #77
)
> will incorporate the patch into our build and do my own tests later. Thanks. I > thought current Webkit was supposed to hit 100/100 on acid3? >
Now that I've tested it, I only get 94/100 and the same "linktest failed" with QtLauncher without the patch.
marcoil
Comment 79
2008-04-24 11:09:13 PDT
(In reply to
comment #76
)
> - Java doesn't load and I'm currently investigating why.
The Java plugin does not work because it depends on OJI, an older interface and not on NPAPI, and this will not be fixed until JRE's next version. It will possibly need extensions to NPAPI, so it should take a while.
Mike Hommey
Comment 80
2008-04-24 11:44:41 PDT
> Now that I've tested it, I only get 94/100 and the same "linktest failed" with > QtLauncher without the patch.
You probably forgot to enable svg support
Adam Williamson
Comment 81
2008-04-25 05:15:59 PDT
Results with the new marcoil patch: acid3 doesn't cause a crash any more. test completes with a score of 98/100 (I'll track down that missing 2 elsewhere, likely nothing to do with the patch). as he noted, scrolling is troublesome. Basically, if you go to Youtube and start playing a video, it'll be fine. If you then scroll the page, the video will freeze on the frame that was being displayed when you scrolled. Audio keeps playing. Thereafter, the video only updates each time you scroll the page: each time you scroll the page the video will update to the 'current' frame, then freeze again.
Adam Williamson
Comment 82
2008-04-25 05:17:59 PDT
Forgot to mention: with the patch, if you just run a browser, go to a 'normal' site and then quit, all is fine. If you run a browser, go to Youtube (probably any other site that causes Flash plugin to be loaded, not tested though) and then quit the browser, it segfaults. Midori just segfaults, Epiphany brings up the GNOME crash window.
marcoil
Comment 83
2008-04-25 05:25:06 PDT
(In reply to
comment #82
)
> Forgot to mention: with the patch, if you just run a browser, go to a 'normal' > site and then quit, all is fine. If you run a browser, go to Youtube (probably > any other site that causes Flash plugin to be loaded, not tested though) and > then quit the browser, it segfaults. Midori just segfaults, Epiphany brings up > the GNOME crash window. >
I've tried it on other sites and it only happens to me on YouTube, curiously. The scrolling bug appeared recently, as I remember it not happening with the original patch.
Sylvain Petreolle
Comment 84
2008-04-25 11:01:36 PDT
(In reply to
comment #83
)
> > I've tried it on other sites and it only happens to me on YouTube, curiously. > > The scrolling bug appeared recently, as I remember it not happening with the > original patch. >
Works beautifully here, nice work. The crash is reproducible on prizee.com, and you can trigger it too when closing the site's tab.
Andrew Badr
Comment 85
2008-04-25 11:38:41 PDT
I'm running WebKit headless (under Xvfb) and experiencing a segfault when I try to load a page with Flash using the latest version of this patch (against
r32489
). Pages without Flash work fine, but a Youtube video for example will crash right away. I'm using the python bindings (pywebkitgtk). Here's the program that segfaults: import webkit, gtk v = webkit.WebView() v.open("
http://www.youtube.com/watch?v=Yz7FFlFy8eM
") gtk.main() Using another URL, such as
http://www.google.com/
, works fine (as youtube did before I installed flash), though there is this warning: GtkWarning: gdk_window_invalidate_rect: assertion `window != NULL' failed (this warning was also present before I patched & installed flash) My uninformed guess is that something is trying to draw to the null window. That code could well be in the Flash plugin. Even if it's in WebKit, I don't expect that this use case is a high priority, but I thought to let you all know anyway.
Gwenole Beauchesne
Comment 86
2008-04-29 04:42:23 PDT
Created
attachment 20887
[details]
64-bit fixes Re-add 64-bit fixes that got lost in recent patches.
Gwenole Beauchesne
Comment 87
2008-04-29 04:45:18 PDT
Created
attachment 20888
[details]
Specialize NPN_GetValue() NPN_GetValue() should be valid for NULL instance and NPNVToolkit or NPNVSupportsXEmbedBool variables. This is the case for Gnash, nspluginwrapper and probably Acrobat Reader (not tested but the comment in npapi.cpp might indicate that).
Gwenole Beauchesne
Comment 88
2008-04-29 04:47:03 PDT
Sorry, individual patches only without respecting proper rules because I am building from the WebKit/Clutter git branch.
marcoil
Comment 89
2008-04-30 04:28:36 PDT
Created
attachment 20901
[details]
Adds NPAPI plugin support for Gtk and Qt-x11,
r32726
This new, updated and improved patch gets youtube videos working without crashes on my setup, on both GtkLauncher and QtLauncher. (In reply to
comment #86
)
> Re-add 64-bit fixes that got lost in recent patches. >
I have incorporated this fix, but not the one separating NPN_GetValue as I'd like to tackle this issue separately. Thanks for the tip!
Adam Williamson
Comment 90
2008-05-01 11:14:04 PDT
With the updated marcoil patch, Youtube scrolling and segfault on exit bugs are both solved. Haven't tested Gwenole's other suggested change yet.
Alp Toker
Comment 91
2008-05-01 11:21:48 PDT
Landed in
r32766
. I had to make a few hundred lines of coding style fixes. Please check the commit to make sure everything is in order. This is going to need some stabilisation work now, still crashes from time to time. We'll probably want it turned off by default in the GTK+ port with a configurable runtime option to enable it, or we're going to be innundated with bug reports.
Maciej Stachowiak
Comment 92
2008-05-02 04:04:27 PDT
Comment on
attachment 20621
[details]
Updated patch against
r31990
- fixed missing files Obsoleting this patch to take the bug out of the review queue.
Gwenole Beauchesne
Comment 93
2008-05-05 00:38:08 PDT
Hi, I have not generated a patch yet, but I got by this one while working on the Clutter version. In WebCore/plugins/gtk/PluginDatabaseGtk.cpp (PluginDatabase::isPreferredPluginDirectory), please use NULL not 0 as the varargs terminator.
Adam Williamson
Comment 94
2008-05-05 07:18:25 PDT
Gwenole, now the support is merged, I think it's better to open new bug reports for issues in the merged code rather than keep adding to this - they'll likely get addressed faster that way.
Gwenole Beauchesne
Comment 95
2008-05-06 04:23:00 PDT
This bug should not have been closed in the first place as per
comment #89
, the NPN_GetValue() regression is not fixed yet. Anyhow, I will open a new bug for this one too. The g_build_filename() thing is in
bug #18906
.
marcoil
Comment 96
2008-05-06 04:28:36 PDT
(In reply to
comment #95
)
> This bug should not have been closed in the first place as per
comment #89
, the > NPN_GetValue() regression is not fixed yet. Anyhow, I will open a new bug for > this one too. The g_build_filename() thing is in
bug #18906
. >
I opened a new bug for
comment #89
,
bug #18892
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug