From 39274b8f605cbedb86e55fc29fd4421e38c717a4 Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Tue, 17 Sep 2019 14:00:42 +0200 Subject: [PATCH 1/7] Required properties: Break binding to model on write This mirrors the behavior in other parts of QML where writing to a property in imperative code breaks the binding. Change-Id: Id19eef17a3c5e77bc4c2772bd749b38c732606a8 Reviewed-by: Simon Hausmann --- src/qmlmodels/qqmldelegatemodel.cpp | 36 ++++++++++- src/qmlmodels/qqmldelegatemodel_p_p.h | 3 + .../data/delegateWithRequiredProperties.3.qml | 64 +++++++++++++++++++ .../qquickpathview/tst_qquickpathview.cpp | 7 ++ 4 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 tests/auto/quick/qquickpathview/data/delegateWithRequiredProperties.3.qml diff --git a/src/qmlmodels/qqmldelegatemodel.cpp b/src/qmlmodels/qqmldelegatemodel.cpp index c37dc114a1..e2a750a43a 100644 --- a/src/qmlmodels/qqmldelegatemodel.cpp +++ b/src/qmlmodels/qqmldelegatemodel.cpp @@ -894,6 +894,7 @@ void PropertyUpdater::doUpdate() auto sender = QObject::sender(); auto mo = sender->metaObject(); auto signalIndex = QObject::senderSignalIndex(); + ++updateCount; // start at 0 instead of propertyOffset to handle properties from parent hierarchy for (auto i = 0; i < mo->propertyCount() + mo->propertyOffset(); ++i) { auto property = mo->property(i); @@ -907,6 +908,27 @@ void PropertyUpdater::doUpdate() } } +void PropertyUpdater::breakBinding() +{ + auto it = senderToConnection.find(QObject::senderSignalIndex()); + if (it == senderToConnection.end()) + return; + if (updateCount == 0) { + QObject::disconnect(*it); + QQmlError warning; + warning.setUrl(qmlContext(QObject::sender())->baseUrl()); + auto signalName = QString::fromLatin1(QObject::sender()->metaObject()->method(QObject::senderSignalIndex()).name()); + signalName.chop(sizeof("changed")-1); + QString propName = signalName; + propName[0] = propName[0].toLower(); + warning.setDescription(QString::fromUtf8("Writing to \"%1\" broke the binding to the underlying model").arg(propName)); + qmlWarning(this, warning); + senderToConnection.erase(it); + } else { + --updateCount; + } +} + void QQDMIncubationTask::initializeRequiredProperties(QQmlDelegateModelItem *modelItemToIncubate, QObject *object) { auto incubatorPriv = QQmlIncubatorPrivate::get(this); @@ -947,10 +969,18 @@ void QQDMIncubationTask::initializeRequiredProperties(QQmlDelegateModelItem *mod // only write to property if it was actually requested by the component if (wasInRequired && prop.hasNotifySignal()) { QMetaMethod changeSignal = prop.notifySignal(); - QMetaMethod updateSlot = PropertyUpdater::staticMetaObject.method(PropertyUpdater::staticMetaObject.indexOfSlot("doUpdate()")); - QObject::connect(modelItemToIncubate, changeSignal, updater, updateSlot); + static QMetaMethod updateSlot = PropertyUpdater::staticMetaObject.method(PropertyUpdater::staticMetaObject.indexOfSlot("doUpdate()")); + QMetaObject::Connection conn = QObject::connect(modelItemToIncubate, changeSignal, updater, updateSlot); + auto propIdx = object->metaObject()->indexOfProperty(propName.toUtf8()); + QMetaMethod writeToPropSignal = object->metaObject()->property(propIdx).notifySignal(); + updater->senderToConnection[writeToPropSignal.methodIndex()] = conn; + static QMetaMethod breakBinding = PropertyUpdater::staticMetaObject.method(PropertyUpdater::staticMetaObject.indexOfSlot("breakBinding()")); + componentProp.write(prop.read(modelItemToIncubate)); + // the connection needs to established after the write, + // else the signal gets triggered by it and breakBinding will remove the connection + QObject::connect(object, writeToPropSignal, updater, breakBinding); } - if (wasInRequired) + else if (wasInRequired) // we still have to write, even if there is no change signal componentProp.write(prop.read(modelItemToIncubate)); } } diff --git a/src/qmlmodels/qqmldelegatemodel_p_p.h b/src/qmlmodels/qqmldelegatemodel_p_p.h index f9dbc61a94..06365a212f 100644 --- a/src/qmlmodels/qqmldelegatemodel_p_p.h +++ b/src/qmlmodels/qqmldelegatemodel_p_p.h @@ -454,8 +454,11 @@ class PropertyUpdater : public QObject public: PropertyUpdater(QObject *parent); + QHash senderToConnection; + int updateCount = 0; public Q_SLOTS: void doUpdate(); + void breakBinding(); }; QT_END_NAMESPACE diff --git a/tests/auto/quick/qquickpathview/data/delegateWithRequiredProperties.3.qml b/tests/auto/quick/qquickpathview/data/delegateWithRequiredProperties.3.qml new file mode 100644 index 0000000000..2996ba18fd --- /dev/null +++ b/tests/auto/quick/qquickpathview/data/delegateWithRequiredProperties.3.qml @@ -0,0 +1,64 @@ +import QtQuick 2.14 + +Item { + id: root + width: 800 + height: 600 + property bool working: false + + ListModel { + id: myModel + ListElement { + name: "Bill Jones" + place: "Berlin" + } + ListElement { + name: "Jane Doe" + place: "Oslo" + } + ListElement { + name: "John Smith" + place: "Oulo" + } + } + + Component { + id: delegateComponent + Rectangle { + id: myDelegate + height: 50 + width: 50 + required property string name + required property int index + onNameChanged: () => {if (myDelegate.name === "You-know-who") root.working = false} + Text { + text: myDelegate.name + font.pointSize: 10 + anchors.fill: myDelegate + } + Component.onCompleted: () => {myDelegate.name = "break binding"} + } + } + + PathView { + anchors.fill: parent + model: myModel + delegate: delegateComponent + path: Path { + startX: 80; startY: 100 + PathQuad { x: 120; y: 25; controlX: 260; controlY: 75 } + PathQuad { x: 140; y: 100; controlX: -20; controlY: 75 } + } + } + Timer { + interval: 1 + running: true + repeat: false + onTriggered: () => { + myModel.setProperty(1, "name", "You-know-who") + myModel.setProperty(2, "name", "You-know-who") + root.working = true + } + } + +} diff --git a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp index 36a242bf3b..d2058cc8b3 100644 --- a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp +++ b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp @@ -2676,6 +2676,13 @@ void tst_QQuickPathView::requiredPropertiesInDelegate() window->show(); QTRY_VERIFY(window->rootObject()->property("working").toBool()); } + { + QScopedPointer window(createView()); + QTest::ignoreMessage(QtMsgType::QtWarningMsg, QRegularExpression("Writing to \"name\" broke the binding to the underlying model")); + window->setSource(testFileUrl("delegateWithRequiredProperties.3.qml")); + window->show(); + QTRY_VERIFY(window->rootObject()->property("working").toBool()); + } } void tst_QQuickPathView::requiredPropertiesInDelegatePreventUnrelated() From be1aa08ad5a0568b580e6a7188f209329a0947de Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 17 Sep 2019 13:34:02 +0200 Subject: [PATCH 2/7] qmllint: Accept arrays as enum descriptions We don't need the values and they are hard to determine statically. Change-Id: I3453e67a18b6d0212748795fd0ea0baaff61b522 Reviewed-by: Simon Hausmann Reviewed-by: Fabian Kosmale --- tools/qmllint/fakemetaobject.cpp | 14 ++---- tools/qmllint/fakemetaobject.h | 3 +- tools/qmllint/qmljstypedescriptionreader.cpp | 51 +++++++++----------- 3 files changed, 28 insertions(+), 40 deletions(-) diff --git a/tools/qmllint/fakemetaobject.cpp b/tools/qmllint/fakemetaobject.cpp index 514bb2fe42..8319ae6713 100644 --- a/tools/qmllint/fakemetaobject.cpp +++ b/tools/qmllint/fakemetaobject.cpp @@ -46,8 +46,8 @@ QString FakeMetaEnum::name() const void FakeMetaEnum::setName(const QString &name) { m_name = name; } -void FakeMetaEnum::addKey(const QString &key, int value) -{ m_keys.append(key); m_values.append(value); } +void FakeMetaEnum::addKey(const QString &key) +{ m_keys.append(key); } QString FakeMetaEnum::key(int index) const { return m_keys.at(index); } @@ -73,10 +73,6 @@ void FakeMetaEnum::addToHash(QCryptographicHash &hash) const hash.addData(reinterpret_cast(&len), sizeof(len)); hash.addData(reinterpret_cast(key.constData()), len * sizeof(QChar)); } - len = m_values.size(); - hash.addData(reinterpret_cast(&len), sizeof(len)); - foreach (int value, m_values) - hash.addData(reinterpret_cast(&value), sizeof(value)); } QString FakeMetaEnum::describe(int baseIndent) const @@ -84,16 +80,14 @@ QString FakeMetaEnum::describe(int baseIndent) const QString newLine = QString::fromLatin1("\n") + QString::fromLatin1(" ").repeated(baseIndent); QString res = QLatin1String("Enum "); res += name(); - res += QLatin1String(":{"); + res += QLatin1String(": ["); for (int i = 0; i < keyCount(); ++i) { res += newLine; res += QLatin1String(" "); res += key(i); - res += QLatin1String(": "); - res += QString::number(m_values.value(i, -1)); } res += newLine; - res += QLatin1Char('}'); + res += QLatin1Char(']'); return res; } diff --git a/tools/qmllint/fakemetaobject.h b/tools/qmllint/fakemetaobject.h index 4e0ea1f8b3..ae76596343 100644 --- a/tools/qmllint/fakemetaobject.h +++ b/tools/qmllint/fakemetaobject.h @@ -46,7 +46,6 @@ namespace LanguageUtils { class FakeMetaEnum { QString m_name; QStringList m_keys; - QList m_values; public: FakeMetaEnum(); @@ -57,7 +56,7 @@ public: QString name() const; void setName(const QString &name); - void addKey(const QString &key, int value); + void addKey(const QString &key); QString key(int index) const; int keyCount() const; QStringList keys() const; diff --git a/tools/qmllint/qmljstypedescriptionreader.cpp b/tools/qmllint/qmljstypedescriptionreader.cpp index 542cdf99eb..44a0d6f8b7 100644 --- a/tools/qmllint/qmljstypedescriptionreader.cpp +++ b/tools/qmllint/qmljstypedescriptionreader.cpp @@ -666,39 +666,34 @@ void TypeDescriptionReader::readEnumValues(AST::UiScriptBinding *ast, LanguageUt return; } - ExpressionStatement *expStmt = AST::cast(ast->statement); + auto *expStmt = AST::cast(ast->statement); if (!expStmt) { - addError(ast->statement->firstSourceLocation(), tr("Expected object literal after colon.")); + addError(ast->statement->firstSourceLocation(), tr("Expected expression after colon.")); return; } - ObjectPattern *objectLit = AST::cast(expStmt->expression); - if (!objectLit) { - addError(expStmt->firstSourceLocation(), tr("Expected object literal after colon.")); - return; - } - - for (PatternPropertyList *it = objectLit->properties; it; it = it->next) { - PatternProperty *assignement = AST::cast(it->property); - if (assignement) { - StringLiteralPropertyName *propName = AST::cast(assignement->name); - NumericLiteral *value = AST::cast(assignement->initializer); - UnaryMinusExpression *minus = AST::cast(assignement->initializer); - if (minus) - value = AST::cast(minus->expression); - if (!propName || !value) { - addError(objectLit->firstSourceLocation(), tr("Expected object literal to contain only 'string: number' elements.")); - continue; + if (auto *objectLit = AST::cast(expStmt->expression)) { + for (PatternPropertyList *it = objectLit->properties; it; it = it->next) { + if (PatternProperty *assignement = it->property) { + if (auto *name = AST::cast(assignement->name)) { + fme->addKey(name->id.toString()); + continue; + } } - - double v = value->value; - if (minus) - v = -v; - fme->addKey(propName->id.toString(), v); - continue; + addError(it->firstSourceLocation(), tr("Expected strings as enum keys.")); } - PatternPropertyList *getterSetter = AST::cast(it->next); - if (getterSetter) - addError(objectLit->firstSourceLocation(), tr("Enum should not contain getter and setters, but only 'string: number' elements.")); + } else if (auto *arrayLit = AST::cast(expStmt->expression)) { + for (PatternElementList *it = arrayLit->elements; it; it = it->next) { + if (PatternElement *element = it->element) { + if (auto *name = AST::cast(element->initializer)) { + fme->addKey(name->value.toString()); + continue; + } + } + addError(it->firstSourceLocation(), tr("Expected strings as enum keys.")); + } + } else { + addError(ast->statement->firstSourceLocation(), + tr("Expected either array or object literal as enum definition.")); } } From 33635f60a708ef2aa7f90c680628878ecadc5168 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 19 Sep 2019 11:15:47 +0200 Subject: [PATCH 3/7] qmllint: Honor --silent argument Change-Id: Ie63654139aeb7bfd241be865f33c399c23e08cc3 Reviewed-by: Fabian Kosmale Reviewed-by: Simon Hausmann --- tools/qmllint/findunqualified.cpp | 37 +++++++++++++------------------ tools/qmllint/findunqualified.h | 3 ++- tools/qmllint/main.cpp | 5 +++-- tools/qmllint/qcoloroutput.cpp | 11 +++++---- tools/qmllint/qcoloroutput_p.h | 2 +- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/tools/qmllint/findunqualified.cpp b/tools/qmllint/findunqualified.cpp index 4404ddf49a..52cc36aeeb 100644 --- a/tools/qmllint/findunqualified.cpp +++ b/tools/qmllint/findunqualified.cpp @@ -40,8 +40,6 @@ #include #include -QDebug operator<<(QDebug dbg, const QQmlJS::AST::SourceLocation &loc); - static QQmlJS::TypeDescriptionReader createReaderForFile(QString const &filename) { QFile f(filename); @@ -139,9 +137,8 @@ void FindUnqualifiedIDVisitor::importHelper(QString id, QString prefix, int majo if (QFile::exists(qmltypesPath)) { auto reader = createReaderForFile(qmltypesPath); auto succ = reader(&objects, &moduleApis, &dependencies); - if (!succ) { - qDebug() << reader.errorMessage(); - } + if (!succ) + m_colorOut.writeUncolored(reader.errorMessage()); break; } } @@ -292,13 +289,17 @@ FindUnqualifiedIDVisitor::localQmlFile2FakeMetaObject(QString filePath) } else if (cast(sourceElement->sourceElement)) { // nothing to do } else { - qDebug() << "unsupportedd sourceElement at" << sourceElement->firstSourceLocation() - << sourceElement->sourceElement->kind; + const auto loc = sourceElement->firstSourceLocation(); + m_colorOut.writeUncolored( + "unsupportedd sourceElement at " + + QString::fromLatin1("%1:%2: ").arg(loc.startLine).arg(loc.startColumn) + + QString::number(sourceElement->sourceElement->kind)); } break; } default: { - qDebug() << "unsupported element of kind" << initMembers->member->kind; + m_colorOut.writeUncolored("unsupported element of kind " + + QString::number(initMembers->member->kind)); } } initMembers = initMembers->next; @@ -368,9 +369,8 @@ bool FindUnqualifiedIDVisitor::visit(QQmlJS::AST::UiProgram *) while (it.hasNext()) { auto reader = createReaderForFile(it.next()); auto succ = reader(&objects, &moduleApis, &dependencies); - if (!succ) { - qDebug() << reader.errorMessage(); - } + if (!succ) + m_colorOut.writeUncolored(reader.errorMessage()); } } // add builtins @@ -578,13 +578,15 @@ bool FindUnqualifiedIDVisitor::visit(QQmlJS::AST::IdentifierExpression *idexp) } FindUnqualifiedIDVisitor::FindUnqualifiedIDVisitor(QStringList const &qmltypeDirs, - const QString &code, const QString &fileName) + const QString &code, const QString &fileName, + bool silent) : m_rootScope(new ScopeTree { ScopeType::JSFunctionScope, "global" }), m_currentScope(m_rootScope.get()), m_qmltypeDirs(qmltypeDirs), m_code(code), m_rootId(QLatin1String("")), - m_filePath(fileName) + m_filePath(fileName), + m_colorOut(silent) { // setup color output m_colorOut.insertColorMapping(Error, ColorOutput::RedForeground); @@ -806,12 +808,3 @@ void FindUnqualifiedIDVisitor::endVisit(QQmlJS::AST::UiObjectDefinition *) { leaveEnvironment(); } - -QDebug operator<<(QDebug dbg, const QQmlJS::AST::SourceLocation &loc) -{ - QDebugStateSaver saver(dbg); - dbg.nospace() << loc.startLine; - dbg.nospace() << ":"; - dbg.nospace() << loc.startColumn; - return dbg.maybeSpace(); -} diff --git a/tools/qmllint/findunqualified.h b/tools/qmllint/findunqualified.h index f7d1aab1f4..98e14f8dc7 100644 --- a/tools/qmllint/findunqualified.h +++ b/tools/qmllint/findunqualified.h @@ -43,7 +43,8 @@ enum class ScopeType; class FindUnqualifiedIDVisitor : public QQmlJS::AST::Visitor { public: - explicit FindUnqualifiedIDVisitor(QStringList const &qmltypeDirs, const QString& code, const QString& fileName); + explicit FindUnqualifiedIDVisitor(QStringList const &qmltypeDirs, const QString& code, + const QString& fileName, bool silent); ~FindUnqualifiedIDVisitor() override; bool check(); diff --git a/tools/qmllint/main.cpp b/tools/qmllint/main.cpp index 235ec16c6e..0fa2ab53e4 100644 --- a/tools/qmllint/main.cpp +++ b/tools/qmllint/main.cpp @@ -50,7 +50,8 @@ static bool lint_file(const QString &filename, const bool silent, const bool war { QFile file(filename); if (!file.open(QFile::ReadOnly)) { - qWarning() << "Failed to open file" << filename << file.error(); + if (!silent) + qWarning() << "Failed to open file" << filename << file.error(); return false; } @@ -76,7 +77,7 @@ static bool lint_file(const QString &filename, const bool silent, const bool war if (success && !isJavaScript && warnUnqualied) { auto root = parser.rootNode(); - FindUnqualifiedIDVisitor v { qmltypeDirs, code, filename}; + FindUnqualifiedIDVisitor v { qmltypeDirs, code, filename, silent }; root->accept(&v); success = v.check(); } diff --git a/tools/qmllint/qcoloroutput.cpp b/tools/qmllint/qcoloroutput.cpp index d2e723700a..eb4c721663 100644 --- a/tools/qmllint/qcoloroutput.cpp +++ b/tools/qmllint/qcoloroutput.cpp @@ -39,7 +39,7 @@ class ColorOutputPrivate { public: - ColorOutputPrivate() : currentColorID(-1) + ColorOutputPrivate(bool silent) : currentColorID(-1), silent(silent) { /* - QIODevice::Unbuffered because we want it to appear when the user actually calls, performance @@ -53,6 +53,7 @@ public: ColorOutput::ColorMapping colorMapping; int currentColorID; bool coloringEnabled; + bool silent; static const char *const foregrounds[]; static const char *const backgrounds[]; @@ -238,7 +239,7 @@ ColorOutput::ColorMapping ColorOutput::colorMapping() const /*! Constructs a ColorOutput instance, ready for use. */ -ColorOutput::ColorOutput() : d(new ColorOutputPrivate()) +ColorOutput::ColorOutput(bool silent) : d(new ColorOutputPrivate(silent)) { } @@ -258,7 +259,8 @@ ColorOutput::~ColorOutput() = default; // must be here so that QScopedPointer ha */ void ColorOutput::write(const QString &message, int colorID) { - d->write(colorify(message, colorID)); + if (!d->silent) + d->write(colorify(message, colorID)); } /*! @@ -269,7 +271,8 @@ void ColorOutput::write(const QString &message, int colorID) */ void ColorOutput::writeUncolored(const QString &message) { - d->write(message + QLatin1Char('\n')); + if (!d->silent) + d->write(message + QLatin1Char('\n')); } /*! diff --git a/tools/qmllint/qcoloroutput_p.h b/tools/qmllint/qcoloroutput_p.h index 710bf5db74..aefa765a87 100644 --- a/tools/qmllint/qcoloroutput_p.h +++ b/tools/qmllint/qcoloroutput_p.h @@ -89,7 +89,7 @@ public: typedef QFlags ColorCode; typedef QHash ColorMapping; - ColorOutput(); + ColorOutput(bool silent); ~ColorOutput(); void setColorMapping(const ColorMapping &cMapping); From 567fc7b40efb451f12eddd9615bae892dcba707f Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 19 Sep 2019 10:42:51 +0200 Subject: [PATCH 4/7] Clean up qmllint test There were several test functions that did effectively the same thing. Unify them. Change-Id: I2d1a9c1534b1c21498c9f0b7a8b80cd4f2a508b5 Reviewed-by: Fabian Kosmale Reviewed-by: Simon Hausmann --- tests/auto/qml/qmllint/tst_qmllint.cpp | 136 +++++++++++++------------ 1 file changed, 71 insertions(+), 65 deletions(-) diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp index 2d225aebd3..9dc0a4dd42 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -38,16 +38,17 @@ class TestQmllint: public QQmlDataTest private Q_SLOTS: void initTestCase() override; - void test(); - void test_data(); + void testUnqualified(); void testUnqualified_data(); + + void cleanQmlCode_data(); + void cleanQmlCode(); + + void dirtyQmlCode_data(); + void dirtyQmlCode(); + void testUnqualifiedNoSpuriousParentWarning(); - void catchIdentifierNoFalsePositive(); - void testUnmatchedSignalHandler(); - void uiQml(); - void methodInScope(); - void importWithPrefix(); private: QString runQmllint(const QString &fileToLint, bool shouldSucceed); @@ -68,24 +69,8 @@ void TestQmllint::initTestCase() } } -void TestQmllint::test_data() -{ - QTest::addColumn("filename"); - QTest::addColumn("isValid"); - - // Valid files: - QTest::newRow("Simple_QML") << QStringLiteral("Simple.qml") << true; - QTest::newRow("QML_importing_JS") << QStringLiteral("importing_js.qml") << true; - QTest::newRow("QTBUG-45916_JS_with_pragma_and_import") << QStringLiteral("QTBUG-45916.js") << true; - - // Invalid files: - QTest::newRow("Invalid_syntax_QML") << QStringLiteral("failure1.qml") << false; - QTest::newRow("Invalid_syntax_JS") << QStringLiteral("failure1.js") << false; -} - void TestQmllint::testUnqualified() { - auto qmlImportDir = QLibraryInfo::location(QLibraryInfo::Qml2ImportsPath); QFETCH(QString, filename); QFETCH(QString, warningMessage); QFETCH(int, warningLine); @@ -118,54 +103,66 @@ void TestQmllint::testUnqualified_data() QTest::newRow("SignalHandlerShort2") << QStringLiteral("SignalHandler.qml") << QStringLiteral("onPressAndHold: (mouse) => {...") << 12 << 34; // access catch identifier outside catch block QTest::newRow("CatchStatement") << QStringLiteral("CatchStatement.qml") << QStringLiteral("err") << 6 << 21; + + QTest::newRow("NonSpuriousParent") << QStringLiteral("nonSpuriousParentWarning.qml") << QStringLiteral("property int x: .parent.x") << 6 << 25; } void TestQmllint::testUnqualifiedNoSpuriousParentWarning() { - runQmllint("spuriousParentWarning.qml", true); - runQmllint("nonSpuriousParentWarning.qml", false); + const QString unknownNotFound = runQmllint("spuriousParentWarning.qml", true); + QVERIFY(unknownNotFound.contains( + QStringLiteral("warning: Unknown was not found. Did you add all import paths?"))); } -void TestQmllint::catchIdentifierNoFalsePositive() +void TestQmllint::dirtyQmlCode_data() { - runQmllint("catchIdentifierNoWarning.qml", true); + QTest::addColumn("filename"); + QTest::addColumn("warningMessage"); + QTest::addColumn("notContained"); + + QTest::newRow("Invalid_syntax_QML") + << QStringLiteral("failure1.qml") + << QStringLiteral("failure1.qml:4 : Expected token `:'") + << QString(); + QTest::newRow("Invalid_syntax_JS") + << QStringLiteral("failure1.js") + << QStringLiteral("failure1.js:4 : Expected token `;'") + << QString(); + QTest::newRow("UnmatchedSignalHandler") + << QStringLiteral("UnmatchedSignalHandler.qml") + << QString("Warning: no matching signal found for handler \"onClicked\" at 12:13") + << QStringLiteral("onMouseXChanged"); } -void TestQmllint::testUnmatchedSignalHandler() -{ - const QString output = runQmllint("UnmatchedSignalHandler.qml", false); - QVERIFY(output.contains(QString::asprintf( - "Warning: no matching signal found for handler \"onClicked\" at %d:%d", 12, 13))); - QVERIFY(!output.contains(QStringLiteral("onMouseXChanged"))); -} - -void TestQmllint::uiQml() -{ - const QString output = runQmllint("FormUser.qml", true); - QVERIFY(output.isEmpty()); -} - -void TestQmllint::methodInScope() -{ - const QString output = runQmllint("MethodInScope.qml", true); - QVERIFY(output.isEmpty()); -} - -void TestQmllint::importWithPrefix() -{ - const QString output = runQmllint("ImportWithPrefix.qml", true); - QVERIFY(output.isEmpty()); -} - -void TestQmllint::test() +void TestQmllint::dirtyQmlCode() { QFETCH(QString, filename); - QFETCH(bool, isValid); - QStringList args; - args << QStringLiteral("--silent") << testFile(filename); + QFETCH(QString, warningMessage); + QFETCH(QString, notContained); - bool success = QProcess::execute(m_qmllintPath, args) == 0; - QCOMPARE(success, isValid); + const QString output = runQmllint(filename, false); + QVERIFY(output.contains(warningMessage)); + if (!notContained.isEmpty()) + QVERIFY(!output.contains(notContained)); +} + +void TestQmllint::cleanQmlCode_data() +{ + QTest::addColumn("filename"); + QTest::newRow("Simple_QML") << QStringLiteral("Simple.qml"); + QTest::newRow("QML_importing_JS") << QStringLiteral("importing_js.qml"); + QTest::newRow("JS_with_pragma_and_import") << QStringLiteral("QTBUG-45916.js"); + QTest::newRow("uiQml") << QStringLiteral("FormUser.qml"); + QTest::newRow("methodInScope") << QStringLiteral("MethodInScope.qml"); + QTest::newRow("importWithPrefix") << QStringLiteral("ImportWithPrefix.qml"); + QTest::newRow("catchIdentifier") << QStringLiteral("catchIdentifierNoWarning.qml"); +} + +void TestQmllint::cleanQmlCode() +{ + QFETCH(QString, filename); + const QString warnings = runQmllint(filename, true); + QVERIFY(warnings.isEmpty()); } QString TestQmllint::runQmllint(const QString &fileToLint, bool shouldSucceed) @@ -173,18 +170,27 @@ QString TestQmllint::runQmllint(const QString &fileToLint, bool shouldSucceed) auto qmlImportDir = QLibraryInfo::location(QLibraryInfo::Qml2ImportsPath); QStringList args; args << QStringLiteral("-U") << testFile(fileToLint) - << QStringLiteral("-I") << qmlImportDir; - QProcess process; - process.start(m_qmllintPath, args); - [&]() { + << QStringLiteral("-I") << qmlImportDir + << QStringLiteral("--silent"); + QString errors; + auto verify = [&](bool isSilent) { + QProcess process; + process.start(m_qmllintPath, args); QVERIFY(process.waitForFinished()); QCOMPARE(process.exitStatus(), QProcess::NormalExit); if (shouldSucceed) QCOMPARE(process.exitCode(), 0); else QVERIFY(process.exitCode() != 0); - }(); - return process.readAllStandardError(); + errors = process.readAllStandardError(); + + if (isSilent) + QVERIFY(errors.isEmpty()); + }; + verify(true); + args.removeLast(); + verify(false); + return errors; } QTEST_MAIN(TestQmllint) From d5f3e7a7e319c6bd8fa8b79d8da7a6fd62b50f01 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 18 Sep 2019 08:45:43 +0200 Subject: [PATCH 5/7] Fix regression when using CONFIG+=QtQuickCompiler and .qrc files in subdirs Commit 41864db3b61d9e81a9fe4906918d2cd3d6d32a0c removed the processing of .qrc files that removed .qml/etc. files. Since the chaining of resources remains critical to ensure that the cached compilation units are loaded, the build system copied the input .qrc file to a new one. That mere copying caused the build to break when the copied .qrc file was not in the same directory as the original one, as file paths within the .qrc file are interpreted as relative to the .qrc file, which was now in a different location. To fix this, this patch brings back the "filtering" code that rewrites the paths to the source files in the .qrc file. Fixes: QTBUG-78253 Change-Id: Ie1d56f3248e713a964260bc2da37c9374f7b6a36 Reviewed-by: Fabian Kosmale Reviewed-by: Ulf Hermann --- .../qml/qmlcachegen/{ => data}/retain.qrc | 2 +- tests/auto/qml/qmlcachegen/qmlcachegen.pro | 2 +- .../auto/qml/qmlcachegen/tst_qmlcachegen.cpp | 3 +- .../Qt5QuickCompilerConfig.cmake.in | 2 +- tools/qmlcachegen/qmlcachegen.cpp | 7 + tools/qmlcachegen/qmlcachegen.pro | 1 + tools/qmlcachegen/qtquickcompiler.prf | 2 +- tools/qmlcachegen/resourcefilter.cpp | 172 ++++++++++++++++++ 8 files changed, 185 insertions(+), 6 deletions(-) rename tests/auto/qml/qmlcachegen/{ => data}/retain.qrc (50%) create mode 100644 tools/qmlcachegen/resourcefilter.cpp diff --git a/tests/auto/qml/qmlcachegen/retain.qrc b/tests/auto/qml/qmlcachegen/data/retain.qrc similarity index 50% rename from tests/auto/qml/qmlcachegen/retain.qrc rename to tests/auto/qml/qmlcachegen/data/retain.qrc index e5eed9b12f..e1b9045fbe 100644 --- a/tests/auto/qml/qmlcachegen/retain.qrc +++ b/tests/auto/qml/qmlcachegen/data/retain.qrc @@ -1,5 +1,5 @@ - data/Retain.qml + Retain.qml diff --git a/tests/auto/qml/qmlcachegen/qmlcachegen.pro b/tests/auto/qml/qmlcachegen/qmlcachegen.pro index 7bd4414302..18d3abd6bc 100644 --- a/tests/auto/qml/qmlcachegen/qmlcachegen.pro +++ b/tests/auto/qml/qmlcachegen/qmlcachegen.pro @@ -25,7 +25,7 @@ workerscripts_test.prefix = /workerscripts RESOURCES += \ workerscripts_test \ trickypaths.qrc \ - retain.qrc + data/retain.qrc # QTBUG-46375 !win32: RESOURCES += trickypaths_umlaut.qrc diff --git a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp index 04f45bb902..7441aed11f 100644 --- a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp +++ b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp @@ -628,8 +628,7 @@ void tst_qmlcachegen::reproducibleCache_data() QTest::addColumn("filePath"); QDir dir(dataDirectory()); - for (const QString &entry : dir.entryList(QDir::Files)) { - QVERIFY(entry.endsWith(".qml") || entry.endsWith(".js") || entry.endsWith(".mjs")); + for (const QString &entry : dir.entryList((QStringList() << "*.qml" << "*.js" << "*.mjs"), QDir::Files)) { QTest::newRow(entry.toUtf8().constData()) << dir.filePath(entry); } } diff --git a/tools/qmlcachegen/Qt5QuickCompilerConfig.cmake.in b/tools/qmlcachegen/Qt5QuickCompilerConfig.cmake.in index baa437a947..be2113b258 100644 --- a/tools/qmlcachegen/Qt5QuickCompilerConfig.cmake.in +++ b/tools/qmlcachegen/Qt5QuickCompilerConfig.cmake.in @@ -50,7 +50,7 @@ but not all the files it references. get_filename_component(input_resource ${_resource} ABSOLUTE) - configure_file(${input_resource} ${new_resource_file} COPYONLY) + execute_process(COMMAND ${compiler_path} --filter-resource-file ${input_resource} -o ${new_resource_file} OUTPUT_VARIABLE remaining_files) list(APPEND filtered_rcc_files ${new_resource_file}) list(APPEND loader_flags \"--resource-file-mapping=${_resource}=${new_resource_file}\") diff --git a/tools/qmlcachegen/qmlcachegen.cpp b/tools/qmlcachegen/qmlcachegen.cpp index 41171b3f07..920f451b84 100644 --- a/tools/qmlcachegen/qmlcachegen.cpp +++ b/tools/qmlcachegen/qmlcachegen.cpp @@ -47,6 +47,7 @@ using namespace QQmlJS; +int filterResourceFile(const QString &input, const QString &output); bool generateLoader(const QStringList &compiledFiles, const QString &output, const QStringList &resourceFileMappings, QString *errorString); QString symbolNamespaceForPath(const QString &relativePath); @@ -417,6 +418,8 @@ int main(int argc, char **argv) parser.addHelpOption(); parser.addVersionOption(); + QCommandLineOption filterResourceFileOption(QStringLiteral("filter-resource-file"), QCoreApplication::translate("main", "Filter out QML/JS files from a resource file that can be cached ahead of time instead")); + parser.addOption(filterResourceFileOption); QCommandLineOption resourceFileMappingOption(QStringLiteral("resource-file-mapping"), QCoreApplication::translate("main", "Path from original resource file to new one"), QCoreApplication::translate("main", "old-name:new-name")); parser.addOption(resourceFileMappingOption); QCommandLineOption resourceOption(QStringLiteral("resource"), QCoreApplication::translate("main", "Qt resource file that might later contain one of the compiled files"), QCoreApplication::translate("main", "resource-file-name")); @@ -462,6 +465,10 @@ int main(int argc, char **argv) if (outputFileName.isEmpty()) outputFileName = inputFile + QLatin1Char('c'); + if (parser.isSet(filterResourceFileOption)) { + return filterResourceFile(inputFile, outputFileName); + } + if (target == GenerateLoader) { ResourceFileMapper mapper(sources); diff --git a/tools/qmlcachegen/qmlcachegen.pro b/tools/qmlcachegen/qmlcachegen.pro index 910cb657d7..bee0b9a37e 100644 --- a/tools/qmlcachegen/qmlcachegen.pro +++ b/tools/qmlcachegen/qmlcachegen.pro @@ -4,6 +4,7 @@ QT = qmldevtools-private DEFINES += QT_NO_CAST_TO_ASCII QT_NO_CAST_FROM_ASCII SOURCES = qmlcachegen.cpp \ + resourcefilter.cpp \ generateloader.cpp \ resourcefilemapper.cpp TARGET = qmlcachegen diff --git a/tools/qmlcachegen/qtquickcompiler.prf b/tools/qmlcachegen/qtquickcompiler.prf index 0129122157..cb3b028e8d 100644 --- a/tools/qmlcachegen/qtquickcompiler.prf +++ b/tools/qmlcachegen/qtquickcompiler.prf @@ -28,7 +28,7 @@ for(res, RESOURCES) { contains(rccContents,.*\\.js$)|contains(rccContents,.*\\.qml$)|contains(rccContents,.*\\.mjs$) { new_resource = $$qmlCacheResourceFileOutputName($$res) mkpath($$dirname(new_resource)) - system($$QMAKE_QMAKE -install qinstall $$system_quote($$absRes) $$system_quote($$new_resource)) + dummy = $$system($$QML_CACHEGEN_FILTER --filter-resource-file -o $$system_quote($$new_resource) $$system_quote($$absRes)) NEWRESOURCES += $$new_resource QMLCACHE_LOADER_FLAGS += --resource-file-mapping=$$shell_quote($$absRes=$$new_resource) diff --git a/tools/qmlcachegen/resourcefilter.cpp b/tools/qmlcachegen/resourcefilter.cpp new file mode 100644 index 0000000000..261102dcbe --- /dev/null +++ b/tools/qmlcachegen/resourcefilter.cpp @@ -0,0 +1,172 @@ +/**************************************************************************** +** +** Copyright (C) 2016 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtQml module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:GPL-EXCEPT$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ +#include +#include +#include +#include + +int filterResourceFile(const QString &input, const QString &output) +{ + enum State { + InitialState, + InRCC, + InResource, + InFile + }; + State state = InitialState; + + QString prefix; + QString currentFileName; + QXmlStreamAttributes fileAttributes; + + QFile file(input); + if (!file.open(QIODevice::ReadOnly)) { + fprintf(stderr, "Cannot open %s for reading.\n", qPrintable(input)); + return EXIT_FAILURE; + } + + QDir inputDirectory = QFileInfo(file).absoluteDir(); + QDir outputDirectory = QFileInfo(output).absoluteDir(); + + QString outputString; + QXmlStreamWriter writer(&outputString); + writer.setAutoFormatting(true); + + QXmlStreamReader reader(&file); + while (!reader.atEnd()) { + switch (reader.readNext()) { + case QXmlStreamReader::StartDocument: { + QStringRef version = reader.documentVersion(); + if (!version.isEmpty()) + writer.writeStartDocument(version.toString()); + else + writer.writeStartDocument(); + break; + } + case QXmlStreamReader::EndDocument: + writer.writeEndDocument(); + break; + case QXmlStreamReader::StartElement: + if (reader.name() == QStringLiteral("RCC")) { + if (state != InitialState) { + fprintf(stderr, "Unexpected RCC tag in line %d\n", int(reader.lineNumber())); + return EXIT_FAILURE; + } + state = InRCC; + } else if (reader.name() == QStringLiteral("qresource")) { + if (state != InRCC) { + fprintf(stderr, "Unexpected qresource tag in line %d\n", int(reader.lineNumber())); + return EXIT_FAILURE; + } + state = InResource; + QXmlStreamAttributes attributes = reader.attributes(); + if (attributes.hasAttribute(QStringLiteral("prefix"))) + prefix = attributes.value(QStringLiteral("prefix")).toString(); + if (!prefix.startsWith(QLatin1Char('/'))) + prefix.prepend(QLatin1Char('/')); + if (!prefix.endsWith(QLatin1Char('/'))) + prefix.append(QLatin1Char('/')); + } else if (reader.name() == QStringLiteral("file")) { + if (state != InResource) { + fprintf(stderr, "Unexpected file tag in line %d\n", int(reader.lineNumber())); + return EXIT_FAILURE; + } + state = InFile; + fileAttributes = reader.attributes(); + continue; + } + writer.writeStartElement(reader.name().toString()); + writer.writeAttributes(reader.attributes()); + continue; + + case QXmlStreamReader::EndElement: + if (reader.name() == QStringLiteral("file")) { + if (state != InFile) { + fprintf(stderr, "Unexpected end of file tag in line %d\n", int(reader.lineNumber())); + return EXIT_FAILURE; + } + state = InResource; + continue; + } else if (reader.name() == QStringLiteral("qresource")) { + if (state != InResource) { + fprintf(stderr, "Unexpected end of qresource tag in line %d\n", int(reader.lineNumber())); + return EXIT_FAILURE; + } + state = InRCC; + } else if (reader.name() == QStringLiteral("RCC")) { + if (state != InRCC) { + fprintf(stderr, "Unexpected end of RCC tag in line %d\n", int(reader.lineNumber())); + return EXIT_FAILURE; + } + state = InitialState; + } + writer.writeEndElement(); + continue; + + case QXmlStreamReader::Characters: + if (reader.isWhitespace()) + break; + if (state != InFile) + return EXIT_FAILURE; + currentFileName = reader.text().toString(); + if (currentFileName.isEmpty()) + continue; + + writer.writeStartElement(QStringLiteral("file")); + + if (!fileAttributes.hasAttribute(QStringLiteral("alias"))) + fileAttributes.append(QStringLiteral("alias"), currentFileName); + + currentFileName = inputDirectory.absoluteFilePath(currentFileName); + currentFileName = outputDirectory.relativeFilePath(currentFileName); + + writer.writeAttributes(fileAttributes); + writer.writeCharacters(currentFileName); + writer.writeEndElement(); + continue; + + default: break; + } + } + + QFile outputFile(output); + if (!outputFile.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + fprintf(stderr, "Cannot open %s for writing.\n", qPrintable(output)); + return EXIT_FAILURE; + } + const QByteArray outputStringUtf8 = outputString.toUtf8(); + if (outputFile.write(outputStringUtf8) != outputStringUtf8.size()) + return EXIT_FAILURE; + + outputFile.close(); + if (outputFile.error() != QFileDevice::NoError) + return EXIT_FAILURE; + + + return EXIT_SUCCESS; +} From 227be82e4cf4b0fc58d4d50154cee7c45eb03777 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 18 Sep 2019 09:12:15 +0200 Subject: [PATCH 6/7] Fix link errors when enabling CONFIG+=qtquickcompiler on non-QML projects Re-apply commit c5578b16d6454e708c8ce12661a85d41eeaaa758 now that with commit 41864db3b61d9e81a9fe4906918d2cd3d6d32a0c the .qml/.js/etc. files are always retained. The difference is that now this is not an error situation but merely a reason to skip processing. Fixes: QTBUG-73669 Change-Id: I53e1e7d471a66e82694ceca83c548c03aaf88f87 Reviewed-by: Ulf Hermann --- tools/qmlcachegen/qtquickcompiler.prf | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/qmlcachegen/qtquickcompiler.prf b/tools/qmlcachegen/qtquickcompiler.prf index cb3b028e8d..a31a7f5714 100644 --- a/tools/qmlcachegen/qtquickcompiler.prf +++ b/tools/qmlcachegen/qtquickcompiler.prf @@ -1,5 +1,15 @@ if(qtc_run|lupdate_run): return() +!contains(QT, qml) { + qt_modules = \ + $$replace(QT, -private$, _private) \ + $$replace(QT_PRIVATE, -private$, _private) + qt_modules = $$resolve_depends(qt_modules, "QT.", ".depends" ".run_depends") + !contains(qt_modules, qml): \ + return() + unset(qt_modules) +} + qtPrepareTool(QML_CACHEGEN, qmlcachegen, _FILTER) qtPrepareTool(QMAKE_RCC, rcc, _DEP) From ab5df626bef9365089ce716ce476bccae1d0a04b Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Thu, 4 Jul 2019 20:05:00 +0200 Subject: [PATCH 7/7] Add dragThreshold property to Event Handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need drag threshold to be adjustable on each handler instance instead of relying only on the system default drag threshold. For example in some use cases DragHandler needs to work with a threshold of 0 or 1 to start dragging as soon as the point is pressed or as soon as the point is moved, with no "jump", to enable fine adjustment of a value on some control such as a Slider. This involves moving the dragOverThreshold() functions that handlers are using from QQuickWindowPrivate to QQuickPointerHandlerPrivate, so that they can use the adjustable threshold value. Task-number: QTBUG-68075 Change-Id: Ie720cbbf9f30abb40d1731d92f8e7f1e6534eeb5 Reviewed-by: Jan Arve Sæther --- src/quick/handlers/qquickdraghandler.cpp | 3 +- src/quick/handlers/qquickpinchhandler.cpp | 7 +- src/quick/handlers/qquickpointerhandler.cpp | 69 +++++++++++++ src/quick/handlers/qquickpointerhandler_p.h | 6 ++ src/quick/handlers/qquickpointerhandler_p_p.h | 7 ++ src/quick/handlers/qquicktaphandler.cpp | 9 +- src/quick/items/qquickitemsmodule.cpp | 3 + src/quick/items/qquickwindow_p.h | 20 ---- .../tst_qquickdraghandler.cpp | 99 ++++++++++++++++++- tests/manual/pointer/pointerDrag.qml | 65 ++++++++++-- 10 files changed, 247 insertions(+), 41 deletions(-) diff --git a/src/quick/handlers/qquickdraghandler.cpp b/src/quick/handlers/qquickdraghandler.cpp index 61b66beff4..492897b68b 100644 --- a/src/quick/handlers/qquickdraghandler.cpp +++ b/src/quick/handlers/qquickdraghandler.cpp @@ -39,6 +39,7 @@ #include "qquickdraghandler_p.h" #include +#include #include QT_BEGIN_NAMESPACE @@ -228,7 +229,7 @@ void QQuickDragHandler::handlePointerEventImpl(QQuickPointerEvent *event) accumulatedDragDelta.setY(0); } qreal angle = std::atan2(accumulatedDragDelta.y(), accumulatedDragDelta.x()) * 180 / M_PI; - bool overThreshold = QQuickWindowPrivate::dragOverThreshold(accumulatedDragDelta); + bool overThreshold = d_func()->dragOverThreshold(accumulatedDragDelta); qCDebug(lcDragHandler) << "movement" << accumulatedDragDelta << "angle" << angle << "of point" << point << "pressed @" << point->scenePressPosition() << "over threshold?" << overThreshold; minAngle = qMin(angle, minAngle); diff --git a/src/quick/handlers/qquickpinchhandler.cpp b/src/quick/handlers/qquickpinchhandler.cpp index a5a867015c..20ad95b0d7 100644 --- a/src/quick/handlers/qquickpinchhandler.cpp +++ b/src/quick/handlers/qquickpinchhandler.cpp @@ -43,6 +43,7 @@ #include #include #include +#include #include #include #include @@ -338,8 +339,7 @@ void QQuickPinchHandler::handlePointerEventImpl(QQuickPointerEvent *event) const QVector2D currentCentroid(centroid().scenePosition()); const QVector2D pressCentroid(centroid().scenePressPosition()); - QStyleHints *styleHints = QGuiApplication::styleHints(); - const int dragThreshold = styleHints->startDragDistance(); + const int dragThreshold = QQuickPointerHandler::dragThreshold(); const int dragThresholdSquared = dragThreshold * dragThreshold; double accumulatedCentroidDistance = 0; // Used to detect scale @@ -399,7 +399,8 @@ void QQuickPinchHandler::handlePointerEventImpl(QQuickPointerEvent *event) point->setAccepted(false); // don't stop propagation setPassiveGrab(point); } - if (QQuickWindowPrivate::dragOverThreshold(point)) + Q_D(QQuickMultiPointHandler); + if (d->dragOverThreshold(point)) ++numberOfPointsDraggedOverThreshold; } diff --git a/src/quick/handlers/qquickpointerhandler.cpp b/src/quick/handlers/qquickpointerhandler.cpp index 12c06aa179..9d4fae1a71 100644 --- a/src/quick/handlers/qquickpointerhandler.cpp +++ b/src/quick/handlers/qquickpointerhandler.cpp @@ -112,6 +112,47 @@ void QQuickPointerHandler::setMargin(qreal pointDistanceThreshold) emit marginChanged(); } +/*! + \qmlproperty int PointerHandler::dragThreshold + \since 5.15 + + The distance in pixels that the user must drag an event point in order to + have it treated as a drag gesture. + + The default value depends on the platform and screen resolution. + It can be reset back to the default value by setting it to undefined. + The behavior when a drag gesture begins varies in different handlers. +*/ +int QQuickPointerHandler::dragThreshold() const +{ + Q_D(const QQuickPointerHandler); + if (d->dragThreshold < 0) + return qApp->styleHints()->startDragDistance(); + return d->dragThreshold; +} + +void QQuickPointerHandler::setDragThreshold(int t) +{ + Q_D(QQuickPointerHandler); + if (d->dragThreshold == t) + return; + + if (t > std::numeric_limits::max()) + qWarning() << "drag threshold cannot exceed" << std::numeric_limits::max(); + d->dragThreshold = qint16(t); + emit dragThresholdChanged(); +} + +void QQuickPointerHandler::resetDragThreshold() +{ + Q_D(QQuickPointerHandler); + if (d->dragThreshold < 0) + return; + + d->dragThreshold = -1; + emit dragThresholdChanged(); +} + /*! Notification that the grab has changed in some way which is relevant to this handler. The \a grabber (subject) will be the Input Handler whose state is changing, @@ -547,4 +588,32 @@ QQuickPointerHandlerPrivate::QQuickPointerHandlerPrivate() { } +template +bool QQuickPointerHandlerPrivate::dragOverThreshold(qreal d, Qt::Axis axis, const TEventPoint *p) const +{ + Q_Q(const QQuickPointerHandler); + QStyleHints *styleHints = qApp->styleHints(); + bool overThreshold = qAbs(d) > q->dragThreshold(); + const bool dragVelocityLimitAvailable = (styleHints->startDragVelocity() > 0); + if (!overThreshold && dragVelocityLimitAvailable) { + qreal velocity = qreal(axis == Qt::XAxis ? p->velocity().x() : p->velocity().y()); + overThreshold |= qAbs(velocity) > styleHints->startDragVelocity(); + } + return overThreshold; +} + +bool QQuickPointerHandlerPrivate::dragOverThreshold(QVector2D delta) const +{ + Q_Q(const QQuickPointerHandler); + const float threshold = q->dragThreshold(); + return qAbs(delta.x()) > threshold || qAbs(delta.y()) > threshold; +} + +bool QQuickPointerHandlerPrivate::dragOverThreshold(const QQuickEventPoint *point) const +{ + QPointF delta = point->scenePosition() - point->scenePressPosition(); + return (dragOverThreshold(delta.x(), Qt::XAxis, point) || + dragOverThreshold(delta.y(), Qt::YAxis, point)); +} + QT_END_NAMESPACE diff --git a/src/quick/handlers/qquickpointerhandler_p.h b/src/quick/handlers/qquickpointerhandler_p.h index c600e42491..995db9c1dc 100644 --- a/src/quick/handlers/qquickpointerhandler_p.h +++ b/src/quick/handlers/qquickpointerhandler_p.h @@ -71,6 +71,7 @@ class Q_QUICK_PRIVATE_EXPORT QQuickPointerHandler : public QObject, public QQmlP Q_PROPERTY(QQuickItem * parent READ parentItem CONSTANT) Q_PROPERTY(GrabPermissions grabPermissions READ grabPermissions WRITE setGrabPermissions NOTIFY grabPermissionChanged) Q_PROPERTY(qreal margin READ margin WRITE setMargin NOTIFY marginChanged) + Q_PROPERTY(int dragThreshold READ dragThreshold WRITE setDragThreshold RESET resetDragThreshold NOTIFY dragThresholdChanged REVISION 15) public: explicit QQuickPointerHandler(QQuickItem *parent = nullptr); @@ -110,11 +111,16 @@ public: qreal margin() const; void setMargin(qreal pointDistanceThreshold); + int dragThreshold() const; + void setDragThreshold(int t); + void resetDragThreshold(); + Q_SIGNALS: void enabledChanged(); void activeChanged(); void targetChanged(); void marginChanged(); + Q_REVISION(15) void dragThresholdChanged(); void grabChanged(QQuickEventPoint::GrabTransition transition, QQuickEventPoint *point); void grabPermissionChanged(); void canceled(QQuickEventPoint *point); diff --git a/src/quick/handlers/qquickpointerhandler_p_p.h b/src/quick/handlers/qquickpointerhandler_p_p.h index 2ea4905643..5727b1ef55 100644 --- a/src/quick/handlers/qquickpointerhandler_p_p.h +++ b/src/quick/handlers/qquickpointerhandler_p_p.h @@ -68,9 +68,16 @@ public: QQuickPointerHandlerPrivate(); + template + bool dragOverThreshold(qreal d, Qt::Axis axis, const TEventPoint *p) const; + + bool dragOverThreshold(QVector2D delta) const; + bool dragOverThreshold(const QQuickEventPoint *point) const; + QQuickPointerEvent *currentEvent = nullptr; QQuickItem *target = nullptr; qreal m_margin = 0; + qint16 dragThreshold = -1; // -1 means use the platform default uint8_t grabPermissions : 8; bool enabled : 1; bool active : 1; diff --git a/src/quick/handlers/qquicktaphandler.cpp b/src/quick/handlers/qquicktaphandler.cpp index a10064a665..5f9d2151d7 100644 --- a/src/quick/handlers/qquicktaphandler.cpp +++ b/src/quick/handlers/qquicktaphandler.cpp @@ -97,13 +97,6 @@ QQuickTapHandler::QQuickTapHandler(QQuickItem *parent) } } -static bool dragOverThreshold(const QQuickEventPoint *point) -{ - QPointF delta = point->scenePosition() - point->scenePressPosition(); - return (QQuickWindowPrivate::dragOverThreshold(delta.x(), Qt::XAxis, point) || - QQuickWindowPrivate::dragOverThreshold(delta.y(), Qt::YAxis, point)); -} - bool QQuickTapHandler::wantsEventPoint(QQuickEventPoint *point) { if (!point->pointerEvent()->asPointerMouseEvent() && @@ -115,7 +108,7 @@ bool QQuickTapHandler::wantsEventPoint(QQuickEventPoint *point) // (e.g. DragHandler) gets a chance to take over. // Don't forget to emit released in case of a cancel. bool ret = false; - bool overThreshold = dragOverThreshold(point); + bool overThreshold = d_func()->dragOverThreshold(point); if (overThreshold) { m_longPressTimer.stop(); m_holdTimer.invalidate(); diff --git a/src/quick/items/qquickitemsmodule.cpp b/src/quick/items/qquickitemsmodule.cpp index f2098f6732..fbfb7521ea 100644 --- a/src/quick/items/qquickitemsmodule.cpp +++ b/src/quick/items/qquickitemsmodule.cpp @@ -497,6 +497,9 @@ static void qt_quickitems_defineModule(const char *uri, int major, int minor) QQuickPointerHandler::tr("ImageBase is an abstract base class")); qmlRegisterType(uri, 2, 14, "Image"); qmlRegisterType(uri, 2, 14, "DragHandler"); + + qmlRegisterUncreatableType(uri, 2, 15, "PointerHandler", + QQuickPointerHandler::tr("PointerHandler is an abstract base class")); } static void initResources() diff --git a/src/quick/items/qquickwindow_p.h b/src/quick/items/qquickwindow_p.h index becbae7fe3..5d44e28bac 100644 --- a/src/quick/items/qquickwindow_p.h +++ b/src/quick/items/qquickwindow_p.h @@ -288,26 +288,6 @@ public: static bool dragOverThreshold(qreal d, Qt::Axis axis, QMouseEvent *event, int startDragThreshold = -1); - template - static bool dragOverThreshold(qreal d, Qt::Axis axis, const TEventPoint *p, int startDragThreshold = -1) - { - QStyleHints *styleHints = qApp->styleHints(); - bool overThreshold = qAbs(d) > (startDragThreshold >= 0 ? startDragThreshold : styleHints->startDragDistance()); - const bool dragVelocityLimitAvailable = (styleHints->startDragVelocity() > 0); - if (!overThreshold && dragVelocityLimitAvailable) { - qreal velocity = axis == Qt::XAxis ? p->velocity().x() : p->velocity().y(); - overThreshold |= qAbs(velocity) > styleHints->startDragVelocity(); - } - return overThreshold; - } - - static bool dragOverThreshold(const QQuickEventPoint *point) - { - QPointF delta = point->scenePosition() - point->scenePressPosition(); - return (QQuickWindowPrivate::dragOverThreshold(delta.x(), Qt::XAxis, point) || - QQuickWindowPrivate::dragOverThreshold(delta.y(), Qt::YAxis, point)); - } - static bool dragOverThreshold(QVector2D delta); // data property diff --git a/tests/auto/quick/pointerhandlers/qquickdraghandler/tst_qquickdraghandler.cpp b/tests/auto/quick/pointerhandlers/qquickdraghandler/tst_qquickdraghandler.cpp index 66314f88a2..65c5ac9ef4 100644 --- a/tests/auto/quick/pointerhandlers/qquickdraghandler/tst_qquickdraghandler.cpp +++ b/tests/auto/quick/pointerhandlers/qquickdraghandler/tst_qquickdraghandler.cpp @@ -53,9 +53,12 @@ private slots: void initTestCase(); void defaultPropertyValues(); + void touchDrag_data(); void touchDrag(); void mouseDrag_data(); void mouseDrag(); + void mouseDragThreshold_data(); + void mouseDragThreshold(); void dragFromMargin(); void snapMode_data(); void snapMode(); @@ -131,9 +134,18 @@ void tst_DragHandler::defaultPropertyValues() QCOMPARE(dragHandler->centroid().sceneGrabPosition(), QPointF()); } +void tst_DragHandler::touchDrag_data() +{ + QTest::addColumn("dragThreshold"); + QTest::newRow("threshold zero") << 0; + QTest::newRow("threshold one") << 1; + QTest::newRow("threshold 20") << 20; + QTest::newRow("threshold default") << -1; +} + void tst_DragHandler::touchDrag() { - const int dragThreshold = QGuiApplication::styleHints()->startDragDistance(); + QFETCH(int, dragThreshold); QScopedPointer windowPtr; createView(windowPtr, "draggables.qml"); QQuickView * window = windowPtr.data(); @@ -142,6 +154,12 @@ void tst_DragHandler::touchDrag() QVERIFY(ball); QQuickDragHandler *dragHandler = ball->findChild(); QVERIFY(dragHandler); + if (dragThreshold < 0) { + dragThreshold = QGuiApplication::styleHints()->startDragDistance(); + QCOMPARE(dragHandler->dragThreshold(), dragThreshold); + } else { + dragHandler->setDragThreshold(dragThreshold); + } QSignalSpy translationChangedSpy(dragHandler, SIGNAL(translationChanged())); QSignalSpy centroidChangedSpy(dragHandler, SIGNAL(centroidChanged())); @@ -161,7 +179,9 @@ void tst_DragHandler::touchDrag() p1 += QPoint(dragThreshold, 0); QTest::touchEvent(window, touchDevice).move(1, p1, window); QQuickTouchUtils::flush(window); - QTRY_VERIFY(dragHandler->centroid().velocity().x() > 0); + qCDebug(lcPointerTests) << "velocity after drag" << dragHandler->centroid().velocity(); + if (dragThreshold > 0) + QTRY_VERIFY(!qFuzzyIsNull(dragHandler->centroid().velocity().x())); QCOMPARE(centroidChangedSpy.count(), 2); QVERIFY(!dragHandler->active()); p1 += QPoint(1, 0); @@ -282,6 +302,81 @@ void tst_DragHandler::mouseDrag() QCOMPARE(centroidChangedSpy.count(), shouldDrag ? 5 : 0); } +void tst_DragHandler::mouseDragThreshold_data() +{ + QTest::addColumn("dragThreshold"); + QTest::newRow("threshold zero") << 0; + QTest::newRow("threshold one") << 1; + QTest::newRow("threshold 20") << 20; + QTest::newRow("threshold default") << -1; +} + +void tst_DragHandler::mouseDragThreshold() +{ + QFETCH(int, dragThreshold); + QScopedPointer windowPtr; + createView(windowPtr, "draggables.qml"); + QQuickView * window = windowPtr.data(); + + QQuickItem *ball = window->rootObject()->childItems().first(); + QVERIFY(ball); + QQuickDragHandler *dragHandler = ball->findChild(); + QVERIFY(dragHandler); + if (dragThreshold < 0) { + dragThreshold = QGuiApplication::styleHints()->startDragDistance(); + QCOMPARE(dragHandler->dragThreshold(), dragThreshold); + } else { + dragHandler->setDragThreshold(dragThreshold); + } + + QSignalSpy translationChangedSpy(dragHandler, SIGNAL(translationChanged())); + QSignalSpy centroidChangedSpy(dragHandler, SIGNAL(centroidChanged())); + + QPointF ballCenter = ball->clipRect().center(); + QPointF scenePressPos = ball->mapToScene(ballCenter); + QPoint p1 = scenePressPos.toPoint(); + QTest::mousePress(window, Qt::LeftButton, Qt::NoModifier, p1); + QVERIFY(!dragHandler->active()); + QCOMPARE(dragHandler->centroid().position(), ballCenter); + QCOMPARE(dragHandler->centroid().pressPosition(), ballCenter); + QCOMPARE(dragHandler->centroid().scenePosition(), scenePressPos); + QCOMPARE(dragHandler->centroid().scenePressPosition(), scenePressPos); + QCOMPARE(dragHandler->centroid().velocity(), QVector2D()); + QCOMPARE(centroidChangedSpy.count(), 1); + p1 += QPoint(dragThreshold, 0); + QTest::mouseMove(window, p1); + if (dragThreshold > 0) + QTRY_VERIFY(dragHandler->centroid().velocity().x() > 0); + QCOMPARE(centroidChangedSpy.count(), 2); + QVERIFY(!dragHandler->active()); + p1 += QPoint(1, 0); + QTest::mouseMove(window, p1); + QTRY_VERIFY(dragHandler->active()); + QCOMPARE(translationChangedSpy.count(), 0); + QCOMPARE(centroidChangedSpy.count(), 3); + QCOMPARE(dragHandler->translation().x(), 0.0); + QPointF sceneGrabPos = p1; + QCOMPARE(dragHandler->centroid().sceneGrabPosition(), sceneGrabPos); + p1 += QPoint(19, 0); + QTest::mouseMove(window, p1); + QTRY_VERIFY(dragHandler->active()); + QCOMPARE(dragHandler->centroid().position(), ballCenter); + QCOMPARE(dragHandler->centroid().pressPosition(), ballCenter); + QCOMPARE(dragHandler->centroid().scenePosition(), ball->mapToScene(ballCenter)); + QCOMPARE(dragHandler->centroid().scenePressPosition(), scenePressPos); + QCOMPARE(dragHandler->centroid().sceneGrabPosition(), sceneGrabPos); + QCOMPARE(dragHandler->translation().x(), dragThreshold + 20.0); + QCOMPARE(dragHandler->translation().y(), 0.0); + QVERIFY(dragHandler->centroid().velocity().x() > 0); + QCOMPARE(centroidChangedSpy.count(), 4); + QTest::mouseRelease(window, Qt::LeftButton, Qt::NoModifier, p1); + QTRY_VERIFY(!dragHandler->active()); + QCOMPARE(dragHandler->centroid().pressedButtons(), Qt::NoButton); + QCOMPARE(ball->mapToScene(ballCenter).toPoint(), p1); + QCOMPARE(translationChangedSpy.count(), 1); + QCOMPARE(centroidChangedSpy.count(), 5); +} + void tst_DragHandler::dragFromMargin() // QTBUG-74966 { const int dragThreshold = QGuiApplication::styleHints()->startDragDistance(); diff --git a/tests/manual/pointer/pointerDrag.qml b/tests/manual/pointer/pointerDrag.qml index 79044eb0b4..2063d8fae2 100644 --- a/tests/manual/pointer/pointerDrag.qml +++ b/tests/manual/pointer/pointerDrag.qml @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2018 The Qt Company Ltd. +** Copyright (C) 2019 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the manual tests of the Qt Toolkit. @@ -26,7 +26,7 @@ ** ****************************************************************************/ -import QtQuick 2.12 +import QtQuick 2.15 import "content" Rectangle { @@ -74,7 +74,14 @@ Rectangle { label: "DragHandler" objectName: "dragSquircle1" DragHandler { - + dragThreshold: ckZeroDragThreshold1.checked ? 0 : undefined + } + CheckBox { + id: ckZeroDragThreshold1 + label: " Zero threshold" + anchors.horizontalCenter: parent.horizontalCenter + y: 20 + checked: false } } @@ -99,7 +106,16 @@ Rectangle { id: tap2 gesturePolicy: root.globalGesturePolicy } - DragHandler { } + DragHandler { + dragThreshold: ckZeroDragThreshold2.checked ? 0 : undefined + } + CheckBox { + id: ckZeroDragThreshold2 + label: " Zero threshold" + anchors.horizontalCenter: parent.horizontalCenter + y: 32 + checked: false + } } TextBox { @@ -107,7 +123,16 @@ Rectangle { width: 100; height: 100 label: "DragHandler\nTapHandler" color: queryColor(tap3.pressed) - DragHandler { } + DragHandler { + dragThreshold: ckZeroDragThreshold3.checked ? 0 : undefined + } + CheckBox { + id: ckZeroDragThreshold3 + label: " Zero threshold" + anchors.horizontalCenter: parent.horizontalCenter + y: 32 + checked: false + } TapHandler { id: tap3 gesturePolicy: root.globalGesturePolicy @@ -118,7 +143,16 @@ Rectangle { x: 400; y: 0 width: 100; height: 100 label: "DragHandler" - DragHandler { } + DragHandler { + dragThreshold: ckZeroDragThreshold4.checked ? 0 : undefined + } + CheckBox { + id: ckZeroDragThreshold4 + label: " Zero threshold" + anchors.horizontalCenter: parent.horizontalCenter + y: 20 + checked: false + } TextBox { label: "TapHandler" @@ -145,6 +179,13 @@ Rectangle { label: " Greedy ↓" checked: true } + CheckBox { + id: ckZeroDragThreshold5 + label: " Zero threshold" + x: 10 + anchors.bottom: ckGreedyDrag.top + checked: false + } TapHandler { id: tap5 gesturePolicy: root.globalGesturePolicy @@ -159,6 +200,7 @@ Rectangle { DragHandler { grabPermissions: ckGreedyDrag ? DragHandler.CanTakeOverFromAnything : DragHandler.CanTakeOverFromItems | DragHandler.CanTakeOverFromHandlersOfDifferentType | DragHandler.ApprovesTakeOverByAnything + dragThreshold: ckZeroDragThreshold5.checked ? 0 : undefined } } } @@ -174,7 +216,16 @@ Rectangle { label: "DragHandler" x: (parent.width - width)/2 y: 60 - DragHandler { } + DragHandler { + dragThreshold: ckZeroDragThreshold6.checked ? 0 : undefined + } + } + CheckBox { + id: ckZeroDragThreshold6 + label: " Zero threshold" + anchors.horizontalCenter: parent.horizontalCenter + y: 20 + checked: false } }