qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix attached property propagation when accessed on QQuickPopupItem 

As mentioned in the comments of QTBUG-68434, popups always inherit
their attributes from the parent window (unless they are explicitly
bound like in ComboBox). However, ComboBox was made an exception,
because the popup is an integral part of the control.

This means that although the following snippet shouldn't change the
background color of a Popup, it should do so for ComboBox's popup:

    Rectangle {
        Material.theme: Material.Dark

        ComboBox {
            model: 5
        }
    }

However, this didn't work.
Before 3ec6d04d97, material/ComboBox.qml
had the following binding in the popup's background Rectangle:

    color: control.popup.Material.dialogColor

That patch changed it in order to get deferred execution working:

    color: parent.Material.dialogColor

This causes the color to come from the window rather than the popup.
This would have almost been caught in test_comboBox, had we checked the
actual color when setting the theme.

To fix the bug without reverting to the previous QML and breaking
deferred execution, we modify findAttachedParent to return the popup's
attached object if the item passed to it is a QQuickPopupItem.

Doing this causes test_inheritance_popup to fail, so we fix that by
passing the attached object target (which can be a popup) to
findAttachedParent, rather than the item whose parent or window changed
(which is coincidentally what was already being done in initialize()).

The patch also adds categorised logging to help debug such issues in
the future. Conveniently, this will also help users who want to debug
their own QQuickAttachedPropertyPropagator subclasses, without them
needing to modify Qt itself.

Fixes: QTBUG-115322
Pick-to: 6.5 6.6
Change-Id: Idc0495023a058bcb033c4002fb6ad233bae9feae
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Mitch Curtis 2023-07-24 17:30:45 +08:00
parent ea63dbfa19
commit c6b401a31a
4 changed files with 91 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
src/quickcontrols/qquickattachedpropertypropagator.cpp
View File

@ -8,9 +8,12 @@
#include <QtQuick/private/qquickitem_p.h>
#include <QtQuick/private/qquickitemchangelistener_p.h>
#include <QtQuickTemplates2/private/qquickpopup_p.h>
#include <QtQuickTemplates2/private/qquickpopupitem_p_p.h>
QT_BEGIN_NAMESPACE
Q_LOGGING_CATEGORY(lcAttached, "qt.quick.controls.attachedpropertypropagator")
/*!
\class QQuickAttachedPropertyPropagator
\brief The QQuickAttachedPropertyPropagator class provides a way to
@ -70,6 +73,33 @@ static QQuickAttachedPropertyPropagator *attachedObject(const QMetaObject *type,
*/
static QQuickAttachedPropertyPropagator *findAttachedParent(const QMetaObject *ourAttachedType, QObject *objectWeAreAttachedTo)
{
/*
In the Material ComboBox.qml, we have code like this:
popup: T.Popup {
// ...
Material.theme: control.Material.theme
// ...
background: Rectangle {
//...
color: parent.Material.dialogColor
The Material attached object has to be accessed this way due to
deferred execution limitations (see 3e87695fb4b1a5d503c744046e6d9f43a2ae18a6).
However, since parent here refers to QQuickPopupItem and not the popup,
the color will actually come from the window. If a dark theme was set on
the ComboBox, it will not be respected in the background if we don't have
the check below.
*/
auto popupItem = qobject_cast<QQuickPopupItem *>(objectWeAreAttachedTo);
if (popupItem) {
auto popupItemPrivate = QQuickPopupItemPrivate::get(popupItem);
QQuickAttachedPropertyPropagator *popupAttached = attachedObject(ourAttachedType, popupItemPrivate->popup);
if (popupAttached)
return popupAttached;
}
QQuickItem *item = qobject_cast<QQuickItem *>(objectWeAreAttachedTo);
if (item) {
// lookup parent items and popups
@ -246,11 +276,16 @@ void QQuickAttachedPropertyPropagatorPrivate::setAttachedParent(QQuickAttachedPr
return;
QQuickAttachedPropertyPropagator *oldParent = attachedParent;
if (attachedParent)
qCDebug(lcAttached) << "setAttachedParent called on" << q->parent();
if (attachedParent) {
qCDebug(lcAttached) << "- removing ourselves as an attached child of" << attachedParent->parent() << attachedParent;
QQuickAttachedPropertyPropagatorPrivate::get(attachedParent)->attachedChildren.removeOne(q);
}
attachedParent = parent;
if (parent)
if (parent) {
qCDebug(lcAttached) << "- adding ourselves as an attached child of" << parent->parent() << parent;
QQuickAttachedPropertyPropagatorPrivate::get(parent)->attachedChildren.append(q);
}
q->attachedParentChange(parent, oldParent);
}
@ -258,9 +293,8 @@ void QQuickAttachedPropertyPropagatorPrivate::itemWindowChanged(QQuickWindow *wi
{
Q_Q(QQuickAttachedPropertyPropagator);
QQuickAttachedPropertyPropagator *attachedParent = nullptr;
QQuickItem *item = qobject_cast<QQuickItem *>(q->sender());
if (item)
attachedParent = findAttachedParent(q->metaObject(), item);
qCDebug(lcAttached) << "window of" << q->parent() << "changed to" << window;
attachedParent = findAttachedParent(q->metaObject(), q->parent());
if (!attachedParent)
attachedParent = attachedObject(q->metaObject(), window);
setAttachedParent(attachedParent);
@ -269,8 +303,9 @@ void QQuickAttachedPropertyPropagatorPrivate::itemWindowChanged(QQuickWindow *wi
void QQuickAttachedPropertyPropagatorPrivate::itemParentChanged(QQuickItem *item, QQuickItem *parent)
{
Q_Q(QQuickAttachedPropertyPropagator);
Q_UNUSED(item);
Q_UNUSED(parent);
setAttachedParent(findAttachedParent(q->metaObject(), item));
setAttachedParent(findAttachedParent(q->metaObject(), q->parent()));
}
/*!
@ -353,8 +388,11 @@ void QQuickAttachedPropertyPropagator::initialize()
d->setAttachedParent(attachedParent);
const QList<QQuickAttachedPropertyPropagator *> attachedChildren = findAttachedChildren(metaObject(), parent());
for (QQuickAttachedPropertyPropagator *child : attachedChildren)
qCDebug(lcAttached) << "initialize called for" << parent() << "- found" << attachedChildren.size() << "attached children";
for (QQuickAttachedPropertyPropagator *child : attachedChildren) {
qCDebug(lcAttached) << "-" << child->parent();
QQuickAttachedPropertyPropagatorPrivate::get(child)->setAttachedParent(this);
}
}
/*!

5
src/quicktemplates/qquickpopupitem.cpp
View File

@ -22,6 +22,11 @@ QQuickPopupItemPrivate::QQuickPopupItemPrivate(QQuickPopup *popup)
isTabFence = true;
}
QQuickPopupItemPrivate *QQuickPopupItemPrivate::get(QQuickPopupItem *popupItem)
{
return popupItem->d_func();
}
void QQuickPopupItemPrivate::implicitWidthChanged()
{
qCDebug(lcPopupItem).nospace() << "implicitWidthChanged called on " << q_func() << "; new implicitWidth is " << implicitWidth;

2
src/quicktemplates/qquickpopupitem_p_p.h
View File

@ -81,6 +81,8 @@ class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickPopupItemPrivate : public QQuickPag
public:
QQuickPopupItemPrivate(QQuickPopup *popup);
static QQuickPopupItemPrivate *get(QQuickPopupItem *popupItem);
void implicitWidthChanged() override;
void implicitHeightChanged() override;

41
tests/auto/quickcontrols/qquickmaterialstyle/data/tst_material.qml
View File

@ -97,6 +97,7 @@ TestCase {
property alias label2: labelInstance2
Popup {
id: popupInstance
objectName: "popupQObject"
Label {
id: labelInstance
text: "test"
@ -549,14 +550,16 @@ TestCase {
window.combo.forceActiveFocus()
verify(window.combo.activeFocus)
keyClick(Qt.Key_Space)
verify(window.combo.popup.visible)
let listView = window.combo.popup.contentItem
let comboPopup = window.combo.popup
verify(comboPopup.visible)
let listView = comboPopup.contentItem
verify(listView)
let child = listView.contentItem.children[0]
verify(child)
compare(window.Material.theme, Material.Light)
compare(window.combo.Material.theme, Material.Dark)
compare(child.Material.theme, Material.Dark)
compare(comboPopup.background.color, comboPopup.Material.dialogColor)
compare(window.Material.primary, Material.color(Material.Blue))
compare(window.combo.Material.primary, Material.color(Material.Blue))
compare(child.Material.primary, Material.color(Material.Blue))
@ -1163,4 +1166,38 @@ TestCase {
compare(button.background.color.a, 0.25)
compare(button.contentItem.color.a, 1)
}
Component {
id: itemPropagationToComboBoxPopup
Rectangle {
id: rectangle
objectName: "rectangle"
anchors.fill: parent
color: "#444"
property alias comboBox: comboBox
Material.theme: Material.Dark
ComboBox {
id: comboBox
objectName: "comboBox"
model: 3
popup.background.objectName: "comboBoxPopupBackground"
popup.contentItem.objectName: "comboBoxPopupContentItem"
}
}
}
function test_itemPropagationToComboBoxPopup() {
let rect = createTemporaryObject(itemPropagationToComboBoxPopup, testCase)
verify(rect)
let comboBox = rect.comboBox
mouseClick(comboBox)
let comboBoxPopup = comboBox.popup
tryCompare(comboBoxPopup, "opened", true)
compare(comboBoxPopup.background.color, comboBoxPopup.Material.dialogColor)
}
}