From e8b3db1bedf1c62f810d26d17ce91a687448bcc4 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Fri, 8 Mar 2019 08:23:16 +0100 Subject: Fix SplitView crash when using certain attached properties If the attached property object was created on an item that SplitView doesn't manage, then its m_splitView member will be null, so check for that. Sometimes, an attached SplitView object will be created on an item that SplitView _does_ manage, but SplitView's own contentItem hasn't been created yet (see the comment in the QQuickSplitViewAttached constructor). In that case the SplitView will see the item added as a child of its contentItem eventually, and we just have to wait. While we are waiting, check access to our members in case they are null. Fixes: QTBUG-74276 Change-Id: I70b7f017e621e0d15c239b962f0407743eb70b15 Reviewed-by: Richard Moe Gustavsen --- src/quicktemplates2/qquicksplitview.cpp | 21 +++++++++----- tests/auto/controls/data/tst_splitview.qml | 44 +++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/quicktemplates2/qquicksplitview.cpp b/src/quicktemplates2/qquicksplitview.cpp index 8aa47987..6d9e983e 100644 --- a/src/quicktemplates2/qquicksplitview.cpp +++ b/src/quicktemplates2/qquicksplitview.cpp @@ -1476,7 +1476,12 @@ QQuickSplitViewAttached::QQuickSplitViewAttached(QObject *parent) { Q_D(QQuickSplitViewAttached); QQuickItem *item = qobject_cast(parent); - if (!item || QQuickItemPrivate::get(item)->isTransparentForPositioner()) + if (!item) { + qmlWarning(parent) << "SplitView: attached properties can only be used on Items"; + return; + } + + if (QQuickItemPrivate::get(item)->isTransparentForPositioner()) return; d->m_splitItem = item; @@ -1637,9 +1642,10 @@ void QQuickSplitViewAttached::setPreferredWidth(qreal width) d->m_isPreferredWidthSet = true; // Make sure that we clear this flag now, before we emit the change signals // which could cause another setter to be called. - auto splitViewPrivate = QQuickSplitViewPrivate::get(d->m_splitView); - const bool ignoreNextLayoutRequest = splitViewPrivate->m_ignoreNextLayoutRequest; - splitViewPrivate->m_ignoreNextLayoutRequest = false; + auto splitViewPrivate = d->m_splitView ? QQuickSplitViewPrivate::get(d->m_splitView) : nullptr; + const bool ignoreNextLayoutRequest = splitViewPrivate && splitViewPrivate->m_ignoreNextLayoutRequest; + if (splitViewPrivate) + splitViewPrivate->m_ignoreNextLayoutRequest = false; if (qFuzzyCompare(width, d->m_preferredWidth)) return; @@ -1705,9 +1711,10 @@ void QQuickSplitViewAttached::setPreferredHeight(qreal height) d->m_isPreferredHeightSet = true; // Make sure that we clear this flag now, before we emit the change signals // which could cause another setter to be called. - auto splitViewPrivate = QQuickSplitViewPrivate::get(d->m_splitView); - const bool ignoreNextLayoutRequest = splitViewPrivate->m_ignoreNextLayoutRequest; - splitViewPrivate->m_ignoreNextLayoutRequest = false; + auto splitViewPrivate = d->m_splitView ? QQuickSplitViewPrivate::get(d->m_splitView) : nullptr; + const bool ignoreNextLayoutRequest = splitViewPrivate && splitViewPrivate->m_ignoreNextLayoutRequest; + if (splitViewPrivate) + splitViewPrivate->m_ignoreNextLayoutRequest = false; if (qFuzzyCompare(height, d->m_preferredHeight)) return; diff --git a/tests/auto/controls/data/tst_splitview.qml b/tests/auto/controls/data/tst_splitview.qml index 8e9522a2..55592eee 100644 --- a/tests/auto/controls/data/tst_splitview.qml +++ b/tests/auto/controls/data/tst_splitview.qml @@ -427,15 +427,45 @@ TestCase { compare(item2.height, testCase.height) } - function test_useAttachedPropertiesIncorrectly() { - var control = createTemporaryObject(splitViewComponent, testCase) - verify(control) + Component { + id: itemComponent + Item {} + } + + Component { + id: objectComponent + QtObject {} + } + + function test_useAttachedPropertiesIncorrectly_data() { + var properties = [ "fillWidth", "fillHeight", "minimumWidth", "minimumHeight", + "preferredWidth", "preferredHeight", "maximumWidth", "maximumHeight" ] + + var data = [] + + for (var i = 0; i < properties.length; ++i) { + var property = properties[i] + data.push({ tag: "Item," + property, component: itemComponent, property: property, + expectedWarning: /.*SplitView: attached properties must be accessed through a direct child of SplitView/ }) + } + + for (i = 0; i < properties.length; ++i) { + property = properties[i] + data.push({ tag: "QtObject," + property, component: objectComponent, property: property, + expectedWarning: /.*SplitView: attached properties can only be used on Items/ }) + } + + return data + } - var item = rectangleComponent.createObject(control, { implicitWidth: 25, color: "salmon" }) - verify(item) + function test_useAttachedPropertiesIncorrectly(data) { + // The object (whatever it may be) is not managed by a SplitView. + var object = createTemporaryObject(data.component, testCase, { objectName: data.tag }) + verify(object) - ignoreWarning(/.*SplitView: attached properties must be accessed through a direct child of SplitView/) - testCase.SplitView.fillWidth = true; + ignoreWarning(data.expectedWarning) + // Should warn, but not crash. + object.SplitView[data.property] = 1; } function test_sizes_data() { -- cgit v1.2.3