Bug 30753 – Build fix for WIN_OS if Netscape plugin support is turned off

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Laszlo Gombos]

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.

[Laszlo Gombos]

Created attachment 41810 [details]
proposed patch

[Kevin Ollivier]

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.

[Laszlo Gombos]

(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

[Kevin Ollivier]

(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?

[Laszlo Gombos]

(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

[Laszlo Gombos]

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.