Bug 24688 - Build fix Symbian; clean Up WebKit/Qt if ENABLE_NETSCAPE_PLUGIN_API = 0
: Build fix Symbian; clean Up WebKit/Qt if ENABLE_NETSCAPE_PLUGIN_API = 0
Status: RESOLVED FIXED
: WebKit
WebKit Qt
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-03-18 17:08 PST by
Modified: 2009-05-11 09:22 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-03-18 17:08:32 PST
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 From 2009-03-18 17:12:50 PST -------
Created an attachment (id=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 From 2009-03-27 09:41:28 PST -------
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 From 2009-03-27 10:58:22 PST -------
Created an attachment (id=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 From 2009-04-29 14:46:29 PST -------
(From update of attachment 28740 [details])
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 From 2009-05-04 12:19:25 PST -------
Simon, Can you please review this or assign it to someone else to review ? Thanks, Laszlo.
------- Comment #6 From 2009-05-11 06:30:39 PST -------
(From update of attachment 29014 [details])
Looks sane to me.
------- Comment #7 From 2009-05-11 09:22:42 PST -------
Landed in r43493.