There are Flash plugins available for DirectFB that can be supported without too much effort in QtWebKit.
I will contribute code for this soon.
Created attachment 51100 [details] Patch that enables Windowless NPAPI for Qt/QWS with DirectFB
Attachment 51100 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/bridge/npapi.h:596: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 51106 [details] Patch that enables Windowless NPAPI for Qt/QWS with DirectFB
Created attachment 51107 [details] Patch that enables Windowless NPAPI for Qt/QWS with DirectFB
Bugs relating to the Qt port of WebKit should have the Qt keyword. See http://trac.webkit.org/wiki/QtWebKitBugs
This patch extends the NPAPI spec by a new platform. I suggest to discuss this addition on the webkit development mailing list first.
Created attachment 56890 [details] Patch that enables Windowless NPAPI for Qt/QWS with DirectFB
Attachment 56890 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/bridge/npapi.h:450: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 56908 [details] Patch that enables Windowless NPAPI for Qt/QWS with DirectFB
Attachment 56908 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/2343151
Created attachment 56942 [details] Patch that enables Windowless NPAPI for Qt/QWS with DirectFB
Attachment 56942 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/plugins/qt/PluginViewQt.cpp:72: Alphabetical sorting problem. [build/include_order] [4] WebCore/bridge/npapi.h:450: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 58046 [details] Patch
Created attachment 63350 [details] Patch
Attachment 63350 [details] did not build on qt: Build output: http://queues.webkit.org/results/3618476
Created attachment 63351 [details] Patch
Created attachment 63382 [details] Patch
It saves reviewers time when you make it explicity what changes from an old to a newer version of your patch.
Created attachment 63384 [details] Patch
Created attachment 63385 [details] Patch
Created attachment 63608 [details] Patch
Comment on attachment 63608 [details] Patch WebCore/bridge/npapi.h:92 + #if defined(XP_UNIX) && !defined(XP_DFB) Doesn't this have to be upstreamed to npapi.h etc used by Mozilla? I think these headers are copied from mozilla, though I might be wrong. WebCore/plugins/PluginView.cpp:366 + #if defined(XP_X11) && !defined(XP_DFB) You are chaning a XP_UNIX to XP_X11, are you sure that is right? I guess XP_UNIX is true on mac. WebCore/plugins/qt/PluginViewQtDFB.cpp:27 + #include "PluginViewQtDFB.h" I guess PluginViewDFBQt would have made more sense WebCore/plugins/qt/PluginViewQtDFB.cpp:48 + extern Q_GUI_EXPORT IDirectFBSurface* qt_directfb_surface_for_pixmap(const QPixmap &pixmap); & is wrongly aligned WebCore/plugins/qt/PluginViewQtDFB.cpp:51 + namespace WebCore { Add a newline after this WebCore/plugins/qt/PluginViewQtDFB.cpp:52 + void PluginViewQtDFB::clearWindowInfo(PluginViewQtDFB::DFBWindowInfo *windowInfo) * is wrongly algined WebCore/plugins/qt/PluginViewQtDFB.cpp:63 + const DFBWindowInfo& windowInfo) put the function definition on one line WebCore/plugins/qt/PluginViewQtDFB.cpp:64 + : m_pluginView(pluginView), m_dfbSurface(0), m_installedEventFilter(false), m_dfbWindowInfo(windowInfo) Put these on separate lines starting with , WebCore/plugins/qt/PluginViewQtDFB.cpp:106 + void PluginViewQtDFB::paint(GraphicsContext* context, const IntRect& rect) a bit of newlines in this method would be nice, grouping the related things together WebCore/plugins/qt/PluginViewQtDFB.cpp:127 + bool PluginViewQtDFB::dispatchEvent(const KeyboardEvent* event) Here as well. This code is hard reading WebCore/plugins/qt/PluginViewQtDFB.cpp:145 + case Qt::Key_Backspace: Can't this be moved to another method? It can always be inline WebCore/plugins/qt/PluginViewQtDFB.cpp:724 + case FocusIn: window.type = DWET_GOTFOCUS; break; dont put this on one line WebCore/plugins/qt/PluginViewQtDFB.cpp:731 + bool PluginViewQtDFB::eventFilter(QObject* , QEvent* e) We normally abbriviate event with 'ev' WebCore/plugins/qt/PluginViewQtDFB.h:40 + class KeyboardEvent; Add a newline after this
Created attachment 63633 [details] Patch
Comment on attachment 63633 [details] Patch This seems very isolated apart from the XP_UNIX -> XP_X11 change which I'm not sure about, so I tending to give you a r=me. Could you please answer my question regarding this before committing? Ah, unfortunately I will have to give you an r-, due to the patch missing files such as the PluginViewQtDFB.cpp etc.
Created attachment 63639 [details] Patch
(In reply to comment #24) > (From update of attachment 63608 [details]) > WebCore/bridge/npapi.h:92 > + #if defined(XP_UNIX) && !defined(XP_DFB) > Doesn't this have to be upstreamed to npapi.h etc used by Mozilla? I think these headers are copied from mozilla, though I might be wrong. > > WebCore/plugins/PluginView.cpp:366 > + #if defined(XP_X11) && !defined(XP_DFB) > You are chaning a XP_UNIX to XP_X11, are you sure that is right? I guess XP_UNIX is true on mac. > > WebCore/plugins/qt/PluginViewQtDFB.cpp:27 > + #include "PluginViewQtDFB.h" > I guess PluginViewDFBQt would have made more sense > > WebCore/plugins/qt/PluginViewQtDFB.cpp:48 > + extern Q_GUI_EXPORT IDirectFBSurface* qt_directfb_surface_for_pixmap(const QPixmap &pixmap); > & is wrongly aligned > > WebCore/plugins/qt/PluginViewQtDFB.cpp:51 > + namespace WebCore { > Add a newline after this > > WebCore/plugins/qt/PluginViewQtDFB.cpp:52 > + void PluginViewQtDFB::clearWindowInfo(PluginViewQtDFB::DFBWindowInfo *windowInfo) > * is wrongly algined > > WebCore/plugins/qt/PluginViewQtDFB.cpp:63 > + const DFBWindowInfo& windowInfo) > put the function definition on one line > > WebCore/plugins/qt/PluginViewQtDFB.cpp:64 > + : m_pluginView(pluginView), m_dfbSurface(0), m_installedEventFilter(false), m_dfbWindowInfo(windowInfo) > Put these on separate lines starting with , > > WebCore/plugins/qt/PluginViewQtDFB.cpp:106 > + void PluginViewQtDFB::paint(GraphicsContext* context, const IntRect& rect) > a bit of newlines in this method would be nice, grouping the related things together > > WebCore/plugins/qt/PluginViewQtDFB.cpp:127 > + bool PluginViewQtDFB::dispatchEvent(const KeyboardEvent* event) > Here as well. This code is hard reading > > WebCore/plugins/qt/PluginViewQtDFB.cpp:145 > + case Qt::Key_Backspace: > Can't this be moved to another method? It can always be inline > > WebCore/plugins/qt/PluginViewQtDFB.cpp:724 > + case FocusIn: window.type = DWET_GOTFOCUS; break; > dont put this on one line > > WebCore/plugins/qt/PluginViewQtDFB.cpp:731 > + bool PluginViewQtDFB::eventFilter(QObject* , QEvent* e) > We normally abbriviate event with 'ev' > > WebCore/plugins/qt/PluginViewQtDFB.h:40 > + class KeyboardEvent; > Add a newline after this Hi Kenneth The XP_X11 was a complete oversight on my part. XP_X11 doesn't really exist. XP_UNIX == X11. Mac is XP_MACOSX. npapi.h in WebKit has diverged a fair bit from the mozilla version. I believe the WebKit occasionally is made functionally equivalent to the Mozilla one by hand.
(In reply to comment #26) > (From update of attachment 63633 [details]) > This seems very isolated apart from the XP_UNIX -> XP_X11 change which I'm not sure about, so I tending to give you a r=me. > > Could you please answer my question regarding this before committing? > > Ah, unfortunately I will have to give you an r-, due to the patch missing files such as the PluginViewQtDFB.cpp etc. Sorry about that. I believe I've addressed all the style issues in the last commit. I didn't see your comment before I committed the second last.
Created attachment 73107 [details] Patch
Attachment 73107 [details] did not build on qt: Build output: http://queues.webkit.org/results/5285023
Comment on attachment 73107 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=73107&action=review Basically, I would like to see all X11 related code guard by x11 defines. > WebCore/bridge/npapi.h:92 > +#if defined(XP_UNIX) && !defined(XP_DFB) I'd prefer if you did: #if defined(XP_UNIX) && defined(X_PROTOCOL) So you'd be including X-related stuff when it is X. > WebCore/bridge/npapi.h:246 > +#if defined(XP_UNIX) && !defined(XP_DFB) Ditto. > WebCore/bridge/npruntime_internal.h:32 > +#if defined XP_UNIX && !defined(XP_DFB) Ditto > WebCore/plugins/PluginView.cpp:373 > +#if defined(XP_X11) && !defined(XP_DFB) here you could do: #if defined(XP_X11_ && PLATFORM(X11)
[CCing Torarne for the PluginViewQt part and Andersca for the npapi changes]
(In reply to comment #33) > [CCing Torarne for the PluginViewQt part and Andersca for the npapi changes] I will comment on this next week, please don't commit as is.
Comment on attachment 73107 [details] Patch I don't like this patch, for a few reasons: - It mixes the new XP_DFB with XP_UNIX. As far as I know the XP-defines are mutually exclusive. XP_UNIX is really XP_X11, and should be treated that way. It should also be defined in npapi.h, and the npapi.h extension should be brought up to a wider audience to make sure it has accept, for example at the plugin-futures list [1]. - It's Qt-specific. If we want to add directfb support we should do it in a way that can be shared between ports, PluginViewDirectFB, where the platform bits are abstracted or #ifdefed. See PluginViewMac for an example. We already have a lot of duplicated code in PluginViewQt and PluginViewGTK, which are both largely implementing what should be PluginViewX11. Let's not make the same mistake with this patch. [1] https://mail.mozilla.org/listinfo/plugin-futures
QtWebKit is gone now.