This patch fixes compilation when QtScript module is disabled.
Created attachment 78982 [details] Made QtScript dependency optional
Attachment 78982 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/bridge/qt/qt_runtime.h:244: Should have a space between // and comment [whitespace/comments] [4] WebKit/qt/Api/qwebelement.cpp:806: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/bridge/qt/qt_instance.cpp:401: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/bridge/qt/qt_class.cpp:228: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/bridge/qt/qt_class.h:61: Should have a space between // and comment [whitespace/comments] [4] WebKit/qt/Api/qwebframe.cpp:633: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/bridge/qt/qt_instance.h:29: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/bridge/qt/qt_instance.h:99: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 8 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 79062 [details] Made QtScript dependency optional
I don't really see us moving in that direction. Right now the only dependency on QtScript we have is that $$prefix/include/QtScript is present. The library does not even have to be compiled! In the future we want to make QtScript a real link-time dependency.
(In reply to comment #4) > I don't really see us moving in that direction. > > Right now the only dependency on QtScript we have is that $$prefix/include/QtScript is present. The library does not even have to be compiled! I realize that, but $$prefix/include/QtScript is not normally installed when script module is not enabled. I've asked for advice on webkit-qt@lists.webkit.org, and Benjamin Poulain adviced me to disable bridge if QtScript is missing
Comment on attachment 79062 [details] Made QtScript dependency optional View in context: https://bugs.webkit.org/attachment.cgi?id=79062&action=review Ok, apart from the other notes I made in the review, I think for this patch to go in we should only disable the parts of the code that actually rely on QScriptEngine::ValueOwnership. One way of doing that would be to disable the QObject bindings, another way would be to introduce a QtWebKit internal enum that mirrors QScriptEngine::ValueOwnership and when QT_NO_SCRIPT is define only the addToJavaScriptWindowObject method overload that takes the QScriptEngine parameter is excluded from the build. The other overload would remain intact. Either way when QT_NO_SCRIPT is define, I think QWebFrame::evaluateJavaScript should remain functional. So let me revert my earlier statement. What you're trying to do makes sense, but I think the patch needs a bit of refinement :) > Source/WebCore/bindings/js/ScriptControllerQt.cpp:54 > +#ifndef QT_NO_SCRIPT > if (widget->isPluginView()) { > PluginView* pluginView = static_cast<PluginView*>(widget); > return pluginView->bindingInstance(); This code - the one to return the binding instance of the PluginView is needed for Netscape plugin scripting to work. The QT_NO_SCRIPT #ifdef has to be placed below that. > Source/WebCore/platform/qt/PlatformMouseEventQt.cpp:39 > +#ifndef QT_NO_GRAPHICSVIEW This should be handled in a different patch.
(In reply to comment #6) > One way of doing that would be to disable the QObject bindings, another way would be to introduce a QtWebKit internal enum that mirrors ScriptEngine::ValueOwnership and when QT_NO_SCRIPT is define only the addToJavaScriptWindowObject method overload that takes the QScriptEngine parameter is excluded from the build. The other overload would remain intact. Which way is more preferable?
(In reply to comment #7) > (In reply to comment #6) > > One way of doing that would be to disable the QObject bindings, another way would be to introduce a QtWebKit internal enum that mirrors ScriptEngine::ValueOwnership and when QT_NO_SCRIPT is define only the > addToJavaScriptWindowObject method overload that takes the QScriptEngine parameter is excluded from the build. The other overload would remain intact. > > Which way is more preferable? Hmm, I prefer the latter, just disabling the overload.
> Hmm, I prefer the latter, just disabling the overload. It requires addition of new public header which should be included instead of <QtScript/qscriptengine.h>, e.g., "scriptvalueownership.h". Where should it be placed (Source/WebCore/bridge/qt, /home/kostya/projects/git/WebKit/Source/WebKit/qt, or somewhere else)? Also, do you have something against separate option for disabling QObject bindings?
*** Bug 49748 has been marked as a duplicate of this bug. ***
*** Bug 34519 has been marked as a duplicate of this bug. ***
=== Bulk closing of Qt bugs === If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary. If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.