Bug 55828

Created attachment 84876 [details] 1st try

(In reply to comment #0) > http://trac.webkit.org/changeset/80007 broke the Symbian build for WK2. To resolve the build-break I propose a simple PLUGIN_ARCHITECTURE(NONE) option. Patch will follow. It broke the Qt on mac build with webkit2 as well. :(

(In reply to comment #2) > (In reply to comment #0) > > http://trac.webkit.org/changeset/80007 broke the Symbian build for WK2. To resolve the build-break I propose a simple PLUGIN_ARCHITECTURE(NONE) option. Patch will follow. > > It broke the Qt on mac build with webkit2 as well. :( Yael, are you suggesting turning on NONE for QtWebKit mac as well ?

(In reply to comment #3) > Yael, are you suggesting turning on NONE for QtWebKit mac as well ? Until we have the mac specific bits for plugins in place, I think we should do that.

I would better like to handle this by the build system mostly because the ENABLE style macros confusing the IDE (i.e. files that guarded with such a macro are ignored to be parsed and indexed). Generally we do it that way for platform specific stuff in WebCore. Besides that PLUGIN_ARCHITECTURE(NONE) doesn't sounds really straightforward.

(In reply to comment #5) > I would better like to handle this by the build system mostly because > the ENABLE style macros confusing the IDE (i.e. files that guarded > with such a macro are ignored to be parsed and indexed). Generally > we do it that way for platform specific stuff in WebCore. > Besides that PLUGIN_ARCHITECTURE(NONE) doesn't sounds really straightforward. After discussed it with Laszlo I realized that my argument against ENABLE style macros is less important than keeping the build system straightforward so I'm ok with this patch. Maybe one day I will fix my problems in qtcreator :)

Created attachment 86666 [details] Patch

This also affects GTK. The new patch add the new files to GNUMakefile.am as well as WebKit2.pro. It has been tested as it is and with adding an "#if 0" before the NETSCAPE_PLUGIN_X11 branch (so it should build and work on Qt-Symbian and Qt-Mac as well).

Created attachment 86701 [details] Patch updated

Comment on attachment 86701 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86701&action=review > Source/WebKit2/ChangeLog:28 > + * WebProcess/Plugins/Netscape/NetscapePluginNone.cpp: Added. wouldn't NetscapePluginEmpty be a better name. That is already used for some similar case in WebKit, just search for *Empty.cpp > Source/WebKit2/ChangeLog:45 > + * config.h: Introduce PLUGIN_ARCHITECTURE(NONE) as another option. What about UNSUPPORTED?

(In reply to comment #10) > (From update of attachment 86701 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=86701&action=review > > > Source/WebKit2/ChangeLog:28 > > + * WebProcess/Plugins/Netscape/NetscapePluginNone.cpp: Added. > > wouldn't NetscapePluginEmpty be a better name. That is already used for some similar case in WebKit, just search for *Empty.cpp Let's grep! $ find Source/ -name "*Empty*" Source/WebCore/WebCore.gyp/mac/Empty.cpp Source/WebCore/platform/mac/EmptyProtocolDefinitions.h Source/WebCore/loader/EmptyClients.h $ find Source/ -name "*None*" Source/JavaScriptCore/wtf/ThreadingNone.cpp Source/WebCore/plugins/PluginDataNone.cpp Source/WebCore/plugins/PluginViewNone.cpp Source/WebCore/plugins/PluginPackageNone.cpp Source/WebCore/platform/text/TextEncodingDetectorNone.cpp Source/WebCore/platform/text/LocalizedNumberNone.cpp Source/WebCore/platform/KillRingNone.cpp This shows me that the name Empty is used in the context of the abstract client pattern while None is used for stubbed implementation. This patch is fall into the second category. > > > Source/WebKit2/ChangeLog:45 > > + * config.h: Introduce PLUGIN_ARCHITECTURE(NONE) as another option. > > What about UNSUPPORTED? Good point. It's a more exact word for this case. Hopefully the mismatch between the macro name and the file name pattern is not really confusing.

Created attachment 86874 [details] Patch

Comment on attachment 86874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86874&action=review LGTM, r=me. Please fix the typo in the changelog before landing. > Source/WebKit2/ChangeLog:48 > + * config.h: Introduce PLUGIN_ARCHITECTURE(UNSOPPORTED) as another nit: s/UNSOPPORTED/UNSUPPORTED

Committed r82577: <http://trac.webkit.org/changeset/82577>