Summary: | Implement NSAPI support for WebKit/Mac inside WebCore/plugins | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tor Arne Vestbø <vestbo> | ||||||||||||||||
Component: | Plug-ins | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | hausmann | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | PC | ||||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||||
Attachments: |
|
Description
Tor Arne Vestbø
2008-10-07 07:00:53 PDT
Created attachment 24143 [details]
Moved implementation of _webkit_isCaseInsensitiveEqualToString to WebCore
Basis for patch 0003
Created attachment 24144 [details]
Move _web_encodingForResource from WebKit into WebCore
Basis for patch 0003
Created attachment 24145 [details]
Implement PluginPackageMac and PluginViewMac
Sorry, was referencing this one as 0003, but Bugzilla seems to remove the patch number.
On a more general note: What we're trying to achieve here is an implementation of NPAPI based on the existing PluginPackage/PluginView code that can be used across all ports that run on Mac OS X. It will take a while to become perfect, but we want to start in small steps. These patches are the basis to continue and implement more in smaller steps. Created attachment 25016 [details]
Rebased against ToT
Created attachment 25017 [details]
Move _web_encodingForResource from WebKit into WebCore and change return type
Rebased against ToT
Created attachment 25019 [details]
Implement PluginPackageMac and PluginViewMac, and enable NPAPI support for QtWebKit/Mac
Rebased against ToT
Comment on attachment 25016 [details]
Rebased against ToT
Looks good to me, but we should probably mention the reason behind this in the ChangeLog
Comment on attachment 25017 [details]
Move _web_encodingForResource from WebKit into WebCore and change return type
Simple patch, looks good to me. We should also mention the reason here.
Comment on attachment 25019 [details] Implement PluginPackageMac and PluginViewMac, and enable NPAPI support for QtWebKit/Mac > From 97e0aaaa77ee021c33364169ad55a8c001573073 Mon Sep 17 00:00:00 2001 > From: =?utf-8?q?Tor=20Arne=20Vestb=C3=B8?= <tavestbo@trolltech.com> > Date: Wed, 17 Sep 2008 16:56:24 +0200 > Subject: [PATCH] =?utf-8?q?2008-11-10=20=20Tor=20Arne=20Vestb=C3=B8=20=20<tavestbo@trolltech.com>?= > MIME-Version: 1.0 > Content-Type: text/plain; charset=utf-8 > Content-Transfer-Encoding: 8bit > > Reviewed by NOBODY (OOPS!). > > Implement PluginPackageMac and PluginViewMac, and enable NPAPI support for QtWebKit/Mac It seems the ChangeLog is missing here. > --- a/WebCore/plugins/PluginView.cpp > +++ b/WebCore/plugins/PluginView.cpp > @@ -138,8 +138,12 @@ void PluginView::frameRectsChanged() const > > void PluginView::handleEvent(Event* event) > { > - if (!m_plugin || m_isWindowed) > + if (!m_plugin) > return; > +#if !defined(XP_MACOSX) > + if (m_isWindowed) > + return; > +#endif This #ifdef probably deserves a comment. > @@ -419,6 +423,12 @@ NPError PluginView::setValue(NPPVariable variable, void* value) > case NPPVpluginTransparentBool: > m_isTransparent = value; > return NPERR_NO_ERROR; > +#if defined(XP_MACOSX) > + case NPPVpluginDrawingModel: > + return NPERR_NO_ERROR; > + case NPPVpluginEventModel: > + return NPERR_NO_ERROR; > +#endif The indentation is off here. > PlatformPluginWidget platformPluginWidget() const { return m_window; } > + void setPlatformPluginWidget(PlatformPluginWidget widget) { > + if (widget != m_window) > + m_window = widget; > + } The assignment is probably faster enough to not require a check for inequality :) > +static int modifiersForEvent(UIEventWithKeyState *event) Misplaced * :) The rest looks good to me Comment on attachment 25019 [details]
Implement PluginPackageMac and PluginViewMac, and enable NPAPI support for QtWebKit/Mac
Ooops, accidentially r+'ed instead of r-. The comments need to be addressed :)
Created attachment 25095 [details]
Updated patch
Fixed comments
Comment on attachment 25095 [details] Updated patch > public: > PlatformPluginWidget platformPluginWidget() const { return m_window; } > + void setPlatformPluginWidget(PlatformPluginWidget widget) { > + m_window = widget; > + } From the looks of it other code in WebCore puts simple inline setters in one line of code. > + if (!dispatchNPEvent(record)) { > + LOG(Events, "PluginView::setFocus(): Get-focus event not accepted"); > + } [...] > + if (!m_isStarted) { > + // TODO: Draw the "missing plugin" image > + return; > + } [...] > + if (!dispatchNPEvent(event)) { > + LOG(Events, "PluginView::paint(): Paint event not accepted"); > + } [...] > + if (!dispatchNPEvent(record)) { > + LOG(Events, "PluginView::handleKeyboardEvent(): Keyboard event type %d not accepted", > + record.what); > + } else { > + event->setDefaultHandled(); > + } [...] > + if (!dispatchNPEvent(record)) { > + LOG(Events, "PluginView::nullEventTimerFired(): Null event not accepted"); > + } All of these one line control clauses should not use braces. I'm really nitpicking here now :-), so r+ under the condition that you fix them before landing. Have these patches all been landed? If so, why is this bug still open? |