QtQml/Connections: Only reconnect signals if target is valid

[stephenmdangelo]

No description provided.

[ChALkeR]

Do you have a testcase for this to compare the behavior with Qt QML?

[stephenmdangelo]

I've added a test case for this. Travis CI seems to have failed for an unrelated reason (seems like just a timing problem).

Qt QML behaviour seems to be broken (at least in Qt 5.4 that I'm testing with). Setting target to null properly disconnects signals when done in a Binding, but not when done via JavaScript.

This works as expected:

import QtQuick 2.0

Item {
    id: root
    property bool should_connect: true
    MouseArea {
        id: mouse_area
        anchors.fill: parent
        Connections {
            id: connections
            target: root.should_connect ? mouse_area : null
            onPressedChanged: {
                console.log("pressed changed")
            }
        }
        onClicked: {
            root.should_connect = !root.should_connect
            console.log("connections target", connections.target)
        }
    }
}

This doesn't:

import QtQuick 2.0

Item {
    MouseArea {
        anchors.fill: parent
        Connections {
            id: connections
            onPressedChanged: {
                console.log("pressed changed")
            }
        }
        onClicked: {
            console.log("connections target", connections.target)
            connections.target = null
            console.log("connections target", connections.target)
        }
    }
}

Documentation says (http://doc.qt.io/qt-5/qml-qtqml-connections.html#target-prop):

If set to null, no connection is made and any signal handlers are ignored until the target is not null.

[ChALkeR]

@stephenmdangelo the Timer test being flaky is a known issue, see #229. I disabled that test in 97bf659.

[ChALkeR]

@stephenmdangelo

Note, this works:

import QtQuick 2.0

Item {
    MouseArea {
        id: mouse_area
        anchors.fill: parent
        Connections {
            id: connections
            target: mouse_area // commenting out this line breaks the test
            onPressedChanged: {
                console.log("pressed changed")
            }
        }
        onClicked: {
            console.log("connections target", connections.target, mouse_area)
            connections.target = null
            console.log("connections target", connections.target, mouse_area)
        }
    }
}

Reported as https://bugreports.qt.io/browse/QTBUG-56499.

[ChALkeR]

LGTM, merging.

[ChALkeR]

@stephenmdangelo, Qt bug was fixed in https://codereview.qt-project.org/173937/.