WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(12.54 KB, patch)
2011-03-23 12:26 PDT
,
Balazs Kelemen
no flags
Details
Formatted Diff
Diff
Patch
(12.64 KB, patch)
2011-03-23 15:12 PDT
,
Balazs Kelemen
no flags
Details
Formatted Diff
Diff
Patch
(12.77 KB, patch)
2011-03-24 20:46 PDT
,
Balazs Kelemen
ossy
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 86666
[details]
Patch
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
Created
attachment 86874
[details]
Patch
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
Committed
r82577
: <
http://trac.webkit.org/changeset/82577
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug