Bug 24688 - Build fix Symbian; clean Up WebKit/Qt if ENABLE_NETSCAPE_PLUGIN_API = 0
Summary: Build fix Symbian; clean Up WebKit/Qt if ENABLE_NETSCAPE_PLUGIN_API = 0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Simon Hausmann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-18 17:08 PDT by Laszlo Gombos
Modified: 2009-05-11 09:22 PDT (History)
4 users (show)

See Also:


Attachments
Proposed fix (3.43 KB, patch)
2009-03-18 17:12 PDT, Laszlo Gombos
no flags Details | Formatted Diff | Diff
Revised patch (3.49 KB, patch)
2009-03-27 10:58 PDT, Laszlo Gombos
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2009-03-18 17:08:32 PDT
ChangeSet r38116 sets ENABLE_NETSCAPE_PLUGIN_API to 0 for Symbian builds; however the ChangeSet fails to turn on the plugin stubs for Symbian (in qt/TemporaryLinkStubs.cpp), so QtLauncher for Symbian fails to link with missing symbols.

In addition ChangeSet r41528 introduced the ability to set ENABLE_NETSCAPE_PLUGIN_API to 0 for all the platforms (including Mac, Win and Linux), but those platforms that enable NETSCAPE_PLUGIN_API support by default will have the same linking problem as the Symbian build.

A patch will follow.
Comment 1 Laszlo Gombos 2009-03-18 17:12:50 PDT
Created attachment 28740 [details]
Proposed fix

Clean ups in addition to fixing the bugs mentioned in the report)
 - All WebKit ports should now share Netscape plugin stubs (introduced by ChangeSet r41786)
 - The decision about Netscape plugin support for Qt is now in one place only (WebCore.pro) - it use to be in the source code as well, and the two were out-of-sync
Comment 2 Simon Hausmann 2009-03-27 09:41:28 PDT
Lazlo, I just tried to build with the patch applied (before landing) and I noticed that it doesn't work entirely:

tmp/PluginViewQt.o: In function `WebCore::PluginView::userAgentStatic()':
PluginViewQt.cpp:(.text+0x60): multiple definition of `WebCore::PluginView::userAgentStatic()'
tmp/TemporaryLinkStubs.o:TemporaryLinkStubs.cpp:(.text+0x0): first defined here
tmp/PluginViewQt.o: In function `WebCore::PluginView::getValueStatic(NPNVariable, void*)':
PluginViewQt.cpp:(.text+0x70): multiple definition of `WebCore::PluginView::getValueStatic(NPNVariable, void*)'
tmp/TemporaryLinkStubs.o:TemporaryLinkStubs.cpp:(.text+0x10): first defined here

I think we may still need the old defines around the two remaining functions in TemporaryLinkStubs.cpp. What do you think?
Comment 3 Laszlo Gombos 2009-03-27 10:58:22 PDT
Created attachment 29014 [details]
Revised patch

Apologies, I do not know how could I miss that.

If I'm not mistaken we can just remove those 2 functions from TemporaryLinkStubs and make the code even simpler.
Comment 4 Eric Seidel (no email) 2009-04-29 14:46:29 PDT
Comment on attachment 28740 [details]
Proposed fix

bugzilla doesn't seem smart enough to remove things from the commit queue when obsolete.  or at least our commit queue query isn't smart enough.
Comment 5 Laszlo Gombos 2009-05-04 12:19:25 PDT
Simon, Can you please review this or assign it to someone else to review ? Thanks, Laszlo.
Comment 6 Eric Seidel (no email) 2009-05-11 06:30:39 PDT
Comment on attachment 29014 [details]
Revised patch

Looks sane to me.
Comment 7 Holger Freyther 2009-05-11 09:22:42 PDT
Landed in r43493.