Bug 30753 - Build fix for WIN_OS if Netscape plugin support is turned off
Summary: Build fix for WIN_OS if Netscape plugin support is turned off
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-24 20:55 PDT by Laszlo Gombos
Modified: 2009-10-25 15:45 PDT (History)
2 users (show)

See Also:


Attachments
proposed patch (4.32 KB, patch)
2009-10-24 20:58 PDT, Laszlo Gombos
no flags 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-10-24 20:55:52 PDT
PLATFORM(WX) guards were placed into the plugin code, because WX port does not yet support Netscape plugins; PLATFORM(WX) guard is misused in this context as there could be other ports without Netscape plugin support. 

I would suggest to consistently use !ENABLE(NETSCAPE_PLUGIN_API) instead of PLATFORM(WX) in the plugin code. THis will make the code easier to read and will resolve some of the build problems when Netscape plugins are not supported or turned off.
Comment 1 Laszlo Gombos 2009-10-24 20:58:35 PDT
Created attachment 41810 [details]
proposed patch
Comment 2 Kevin Ollivier 2009-10-25 08:33:19 PDT
I'd prefer not to do this because we intend to support plugins in the not-too-distant future (i.e. I intend to devote some serious time to it in the next couple months and actually have some code in a git branch as a start on it), and in the meantime, it's convenient that we get notified of any breakages as they happen rather than trying to figure out why some change doesn't work with the wx port 3-4 months after the fact.
Comment 3 Laszlo Gombos 2009-10-25 08:56:44 PDT
(In reply to comment #2)
> I'd prefer not to do this because we intend to support plugins in the
> not-too-distant future (i.e. I intend to devote some serious time to it in the
> next couple months and actually have some code in a git branch as a start on
> it), and in the meantime, it's convenient that we get notified of any breakages
> as they happen rather than trying to figure out why some change doesn't work
> with the wx port 3-4 months after the fact.

Ollivier, I would say this change will help to turn on plugin support for wx port. In the meantime wx port can set ENABLE_NETSCAPE_PLUGIN_API to 0 (as other ports, like the QtWebKit Symbian port used to). 

I missed the fact that wx port did not already set ENABLE_NETSCAPE_PLUGIN_API to 0. Do you have a preference where to set ENABLE_NETSCAPE_PLUGIN_API to 0 for wx port (bkl files or Platform.h or some other pace) ? Thanks. Laszlo
Comment 4 Kevin Ollivier 2009-10-25 10:18:16 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > I'd prefer not to do this because we intend to support plugins in the
> > not-too-distant future (i.e. I intend to devote some serious time to it in the
> > next couple months and actually have some code in a git branch as a start on
> > it), and in the meantime, it's convenient that we get notified of any breakages
> > as they happen rather than trying to figure out why some change doesn't work
> > with the wx port 3-4 months after the fact.
> 
> Ollivier, I would say this change will help to turn on plugin support for wx
> port. 

How? I don't really understand the logic behind this change.

> In the meantime wx port can set ENABLE_NETSCAPE_PLUGIN_API to 0 (as other
> ports, like the QtWebKit Symbian port used to). 
>
> I missed the fact that wx port did not already set ENABLE_NETSCAPE_PLUGIN_API
> to 0. Do you have a preference where to set ENABLE_NETSCAPE_PLUGIN_API to 0 for
> wx port (bkl files or Platform.h or some other pace) ? Thanks. Laszlo

It is intentionally not set. Maybe you are not clear on this - I *want* ENABLE_NETSCAPE_PLUGIN_API set to 1, not 0. The platform APIs are not yet implemented, but they will be, and in the meantime, the stubbed implementation compiles just fine. What is wrong with that?

Could you explain what problem you're trying to address? Does wx's ability to compile with ENABLE_NETSCAPE_PLUGIN_API support on cause problems for you?
Comment 5 Laszlo Gombos 2009-10-25 15:32:13 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > I'd prefer not to do this because we intend to support plugins in the
> > > not-too-distant future (i.e. I intend to devote some serious time to it in the
> > > next couple months and actually have some code in a git branch as a start on
> > > it), and in the meantime, it's convenient that we get notified of any breakages
> > > as they happen rather than trying to figure out why some change doesn't work
> > > with the wx port 3-4 months after the fact.
> > 
> > Ollivier, I would say this change will help to turn on plugin support for wx
> > port. 
> 
> How? I don't really understand the logic behind this change.
> 
> > In the meantime wx port can set ENABLE_NETSCAPE_PLUGIN_API to 0 (as other
> > ports, like the QtWebKit Symbian port used to). 
> >
> > I missed the fact that wx port did not already set ENABLE_NETSCAPE_PLUGIN_API
> > to 0. Do you have a preference where to set ENABLE_NETSCAPE_PLUGIN_API to 0 for
> > wx port (bkl files or Platform.h or some other pace) ? Thanks. Laszlo
> 
> It is intentionally not set. Maybe you are not clear on this - I *want*
> ENABLE_NETSCAPE_PLUGIN_API set to 1, not 0. The platform APIs are not yet
> implemented, but they will be, and in the meantime, the stubbed implementation
> compiles just fine. What is wrong with that?
> 
> Could you explain what problem you're trying to address? Does wx's ability to
> compile with ENABLE_NETSCAPE_PLUGIN_API support on cause problems for you?

You're right, I though Netscape plugin support is turned off for wx port. 

My particular problem was turning off Netscape plugin support for WIN_OS. I changed the bug title to say just that and will change the patch to address just that particular problem; and will not change the PLATFORM(WX) guards. 

Thanks for helping fixing my patch. Laszlo
Comment 6 Laszlo Gombos 2009-10-25 15:45:19 PDT
After addressing only the WIN_OS specific problem the patch become trivial build fix. Committed as http://trac.webkit.org/changeset/50047. Closing the bug.