setParentItem sets visual parent. Object parent and visual parent can be different.

I have a feeling that if we change

const QList<QQuickItem*> items = d->window()->contentItem()->findChildren<QQuickItem*>(QString(), Qt::FindDirectChildrenOnly);

const QList<QQuickItem*> items = d->window()->contentItem()->childItems();

arrow keys will work.

Maybe, it should be fixed in Plasma::Dialog instead?

I confirmed that contentItem()->childItems() did indeed send it to mainItem. It also sends it to the FrameSvgItem too. It no longer sends it to the mysterious ColorScope.

kwin_tabbox:     d->window() PlasmaQuick::Dialog(0x11927d0 exposed, ...)
kwin_tabbox:     d->window()->contentItem() QQuickRootItem(0x117b820, parent=0x0, geometry=0,0 1240x404)
kwin_tabbox:     d->window()->sendEvent Plasma::FrameSvgItem(0x11a5910, parent=0x117b820, geometry=0,0 1240x404)
kwin_tabbox:     d->window()->sendEvent QQuickItem_QML_52(0x10f2090, parent=0x117b820, geometry=4,4 1232x396)

Maybe, it should be fixed in Plasma::Dialog instead?

That's what I was afraid of.

Thanks for telling me about the differences of QQuickItem::setParentItem(). Totally missed that. So I assume that is suppose to be QObject::setParent()? Changing dialog.cpp would require a crapload of testing.

I decided to check what mainItem.parent is right now, and it looks like it's QQuickRootItem. A normal Item defined in a property has a parent of null.

PlasmaCore.Dialog {
	id: dialog
	mainItem: Item {
		id: dialogMainItem

		Component.onCompleted: {
			// Before:
			//   dialogMainItem.parent = ?
			//   dialogMainItem.parentItem = dialog.contentItem()

			// qml: dialog PlasmaQuick::Dialog(0xd8e650)
			// qml: dialogMainItem QQuickItem_QML_49(0xdf3d00)
			// qml: dialogMainItem.parent QQuickRootItem(0xd84180)

			// After:
			//   dialogMainItem.parent = dialog.contentItem()
			//   dialogMainItem.parentItem = ?

			// ?
		}
	}

	property Item testItem: Item {
		id: dialogTestItem
		Component.onCompleted: {
			console.log('dialogTestItem', dialogTestItem)
			console.log('dialogTestItem.parent', dialogTestItem.parent)
			// qml: dialogTestItem QQuickItem(0x101cf70)
			// qml: dialogTestItem.parent null
		}
	}
}

What's the status of this patch? I'd love to get this fix, because it's a blocker for https://bugs.kde.org/show_bug.cgi?id=308331.

I need to try patching plasma-framework's dialog.cpp and see if that introduces issues. Right now it's mainItem->setParentItem(contentItem());, and I need to test if mainItem->setParent(contentItem()); breaks anything. I haven't patched C++ in frameworks before so I'll need to figure that out.

https://github.com/KDE/plasma-framework/blob/master/src/plasmaquick/dialog.cpp#L797

If I look at the git blame of that line, the last commit that touched setParentItem shows that there used to be a d->mainItem->setParent(parent());

https://github.com/KDE/plasma-framework/commit/afe0524fa7371fe8e3f83010fd3fc06b6f9adc77#diff-af30a0e9bd2695def291bffd8524c589R636

Hmmm, it looks like setParent was removed in this commit.

https://github.com/KDE/plasma-framework/commit/ad03d0bb145736f26b8782f1229014172512084e

@mart: Should I add main mainItem->setParent(contentItem());, or should we fix this in KWin?

Don't focus too much on Plasma::Dialog. It should also work with a plain QQuickWindow or a QuickControls2::Dialog

Both old and patched code seem weird.

I would expect to send it to window->activeFocusItem
(or just send the event to the window which will then do that)

It will then match what a normal QML user would expect.
That /should/ work for at least the main breeze theme (untested)

@mart: Should I add main mainItem->setParent(contentItem());, or should we fix this in KWin?

Not having mainItem a visual child of the content item is super weird - but changing anything in Dialog is risky.

A reason for the old code being weird is that KWin is weird. I remember that I fought quite a lot when porting this to Qt5 and there was a time when it was working.