RESOLVED FIXED 55828
[WK2] Introduce an option for no NPAPI support
https://bugs.webkit.org/show_bug.cgi?id=55828
Summary [WK2] Introduce an option for no NPAPI support
Laszlo Gombos
Reported 2011-03-05 13:31:09 PST
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.
Attachments
1st try (5.49 KB, patch)
2011-03-05 14:51 PST, Laszlo Gombos
no flags
Patch (12.54 KB, patch)
2011-03-23 12:26 PDT, Balazs Kelemen
no flags
Patch (12.64 KB, patch)
2011-03-23 15:12 PDT, Balazs Kelemen
no flags
Patch (12.77 KB, patch)
2011-03-24 20:46 PDT, Balazs Kelemen
ossy: review+
Laszlo Gombos
Comment 1 2011-03-05 14:51:33 PST
Created attachment 84876 [details] 1st try
Yael
Comment 2 2011-03-05 14:54:59 PST
(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. :(
Laszlo Gombos
Comment 3 2011-03-05 15:45:19 PST
(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 ?
Yael
Comment 4 2011-03-05 18:14:52 PST
(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.
Balazs Kelemen
Comment 5 2011-03-06 04:17:26 PST
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.
Balazs Kelemen
Comment 6 2011-03-22 08:37:32 PDT
(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 :)
Balazs Kelemen
Comment 7 2011-03-23 12:26:42 PDT
Balazs Kelemen
Comment 8 2011-03-23 12:31:09 PDT
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).
Balazs Kelemen
Comment 9 2011-03-23 15:12:38 PDT
Created attachment 86701 [details] Patch updated
Kenneth Rohde Christiansen
Comment 10 2011-03-23 15:19:46 PDT
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?
Balazs Kelemen
Comment 11 2011-03-24 07:01:39 PDT
(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.
Balazs Kelemen
Comment 12 2011-03-24 20:46:18 PDT
Csaba Osztrogonác
Comment 13 2011-03-31 07:17:19 PDT
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
Balazs Kelemen
Comment 14 2011-03-31 07:55:03 PDT
Note You need to log in before you can comment on or make changes to this bug.