Summary: | [Qt] Refactor QtWebKitPlatformPlugin interface | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tor Arne Vestbø <vestbo> | ||||
Component: | New Bugs | Assignee: | Tor Arne Vestbø <vestbo> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, ademar, eric, hausmann, kenneth, luiz, raine.makelainen, webkit.review.bot, yael | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Bug Depends on: | 46402 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Tor Arne Vestbø
2010-09-23 03:48:03 PDT
Created attachment 68507 [details]
Patch
Committed r68128: <http://trac.webkit.org/changeset/68128> http://trac.webkit.org/changeset/68128 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release The following changes are on the blame list: http://trac.webkit.org/changeset/68128 http://trac.webkit.org/changeset/68129 Commited r68130: <http://trac.webkit.org/changeset/68130> Cherry-picking it to 2.1 requires a backport due to conflicts with r64829 <http://trac.webkit.org/changeset/64829>, Bug 43427 The most important change affecting the cherry-pick appears to be the change of the createSelectInputMethod() signature in r64829: (WebKit/qt/WebCoreSupport/QtPlatformPlugin.cpp) -QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod() +QWebSelectMethod* QtPlatformPlugin::createSelectInputMethod() BTW, r64829 is a large patch and trying to cherry-pick it I see even more conflicts (6 conflicting files). Apparently it requires the changes from Bug 42592 (also a large patch, with 9 file conflicts when cherry-picking to 2.1) /o\ :-) Does it make sense to backport only this patch to 2.1, without the previous changes? (In reply to comment #5) > Cherry-picking it to 2.1 requires a backport due to conflicts with r64829 <http://trac.webkit.org/changeset/64829>, Bug 43427 > > The most important change affecting the cherry-pick appears to be the change of the createSelectInputMethod() signature in r64829: > > (WebKit/qt/WebCoreSupport/QtPlatformPlugin.cpp) > -QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod() > +QWebSelectMethod* QtPlatformPlugin::createSelectInputMethod() > > BTW, r64829 is a large patch and trying to cherry-pick it I see even more conflicts (6 conflicting files). Apparently it requires the changes from Bug 42592 (also a large patch, with 9 file conflicts when cherry-picking to 2.1) > > /o\ :-) > > Does it make sense to backport only this patch to 2.1, without the previous changes? I think that in 2.1 it should be: QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod() { QWebKitPlatformPlugin* p = plugin(); if (!p) return 0; - QWebSelectMethod* selector = p->createSelectInputMethod(); + QWebSelectMethod* selector =qobject_cast<QWebSelectMethod*>(p->createExtension(QWebKitPlatformPlugin::MultipleSelections)) : 0; if (!selector) return 0; return new SelectInputMethodWrapper(selector); } (In reply to comment #6) > (In reply to comment #5) > > Cherry-picking it to 2.1 requires a backport due to conflicts with r64829 <http://trac.webkit.org/changeset/64829>, Bug 43427 > > > > The most important change affecting the cherry-pick appears to be the change of the createSelectInputMethod() signature in r64829: > > > > (WebKit/qt/WebCoreSupport/QtPlatformPlugin.cpp) > > -QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod() > > +QWebSelectMethod* QtPlatformPlugin::createSelectInputMethod() > > > > BTW, r64829 is a large patch and trying to cherry-pick it I see even more conflicts (6 conflicting files). Apparently it requires the changes from Bug 42592 (also a large patch, with 9 file conflicts when cherry-picking to 2.1) > > > > /o\ :-) > > > > Does it make sense to backport only this patch to 2.1, without the previous changes? > > I think that in 2.1 it should be: > > QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod() > { > QWebKitPlatformPlugin* p = plugin(); > if (!p) > return 0; > > - QWebSelectMethod* selector = p->createSelectInputMethod(); > + QWebSelectMethod* selector =qobject_cast<QWebSelectMethod*>(p->createExtension(QWebKitPlatformPlugin::MultipleSelections)) : 0; > > if (!selector) > return 0; > > return new SelectInputMethodWrapper(selector); > } please ignore the ": 0" in inserted line in the previous comment. :) the correct line is: + QWebSelectMethod* selector =qobject_cast<QWebSelectMethod*>(p->createExtension(QWebKitPlatformPlugin::MultipleSelections)); Besides the already discussed change on QtPlatformPlugin::createSelectInputMethod(), we also need the patch from Bug 46402 and the following change: --- a/WebCore/platform/qt/PlatformHapticsQt.cpp +++ b/WebCore/platform/qt/PlatformHapticsQt.cpp @@ -33,7 +33,7 @@ void initHaptics() if (!pluginLoaded) { if (platformPlugin.plugin() && platformPlugin.plugin()->supportsExtension(QWebKitPlatformPlugin::Haptics)) - player = platformPlugin.plugin()->createHapticFeedbackPlayer(); + player = qobject_cast<QWebHapticFeedbackPlayer*>(platformPlugin.plugin()->createExtension(QWebKitPlatformPlugin::Haptics)); pluginLoaded = true; } } With those changes I can build and run QtTestBrowser - although I couldn't manage to test the new plugin interface. Anything else? Does somebody has a testcase or a plugin I can use to test? (In reply to comment #8) > Besides the already discussed change on QtPlatformPlugin::createSelectInputMethod(), we also need the patch from Bug 46402 and the following change: > > --- a/WebCore/platform/qt/PlatformHapticsQt.cpp > +++ b/WebCore/platform/qt/PlatformHapticsQt.cpp > @@ -33,7 +33,7 @@ void initHaptics() > > if (!pluginLoaded) { > if (platformPlugin.plugin() && platformPlugin.plugin()->supportsExtension(QWebKitPlatformPlugin::Haptics)) > - player = platformPlugin.plugin()->createHapticFeedbackPlayer(); > + player = qobject_cast<QWebHapticFeedbackPlayer*>(platformPlugin.plugin()->createExtension(QWebKitPlatformPlugin::Haptics)); > pluginLoaded = true; > } > } > This part should ideally be refactored into the WebCore-side helper QtPlatformPlugin like the others, so the qobject_cast is hidden. (In reply to comment #9) > (In reply to comment #8) > <snip> > --- a/WebCore/platform/qt/PlatformHapticsQt.cpp > +++ b/WebCore/platform/qt/PlatformHapticsQt.cpp <snip> > > This part should ideally be refactored into the WebCore-side helper QtPlatformPlugin like the others, so the qobject_cast is hidden. Sure. It also applies to trunk. Bug 46402 has been updated (includes patch) Revision r68128 cherry-picked into qtwebkit-2.1 with commit 9b5a4b4 <http://gitorious.org/webkit/qtwebkit/commit/9b5a4b4> Revision r68130 cherry-picked into qtwebkit-2.1 with commit 0f90c4b <http://gitorious.org/webkit/qtwebkit/commit/0f90c4b> |