Skip to content

fix: 关闭弹窗时同时关闭启动器 #1397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Ivy233 wants to merge 1 commit into from
Closed

fix: 关闭弹窗时同时关闭启动器 #1397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion panels/dock/MenuHelper.qml
View file
Open in desktop
Copy link
Member

@BLumia BLumia Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onRequestClosePopup 的时候关掉启动器的话，下面那一堆仍然是需要的吗？

Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,30 @@ import Qt.labs.platform

Item {
property Menu activeMenu: null
property var aboutToHideConnections: ({}) // Store connections by menu object
Copy link

@sourcery-ai sourcery-ai bot Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Using a plain JS object keyed by menu instances is unreliable because keys are coerced to strings.

Because QML/JS object keys are always strings, indexing with aboutToHideConnections[menu] will stringify menu (e.g. to "[object Object]"), so different Menu instances can collide and overwrite each other’s handlers. Instead, store the handler directly on the menu instance (e.g. menu._aboutToHideHandler) or use a small helper component that keeps the connection alongside the specific Menu object.

Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a Menu object as a JavaScript object key can lead to unexpected behavior because QML objects don't have stable string representations. This could cause memory leaks as old menu instances won't be properly cleaned up from the dictionary. Consider using a WeakMap-like pattern or implementing cleanup logic in a Component.onDestruction handler.

Copilot uses AI. Check for mistakes.

function openMenu(menu: Menu) {
if (activeMenu) {
activeMenu.close()
}

// Disconnect previous connection for this menu if exists
if (aboutToHideConnections[menu]) {
try {
menu.aboutToHide.disconnect(aboutToHideConnections[menu])
} catch (e) {
// Silently ignore disconnect errors
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try-catch block silently suppresses disconnect errors without logging or clearing the stale entry from aboutToHideConnections. If disconnection fails, the stale handler remains in the dictionary, potentially causing memory leaks. Consider logging the error or removing the entry from aboutToHideConnections after the disconnect attempt.

Suggested change
// Silently ignore disconnect errors
console.warn("Failed to disconnect aboutToHide handler for menu:", menu, "error:", e)
} finally {
delete aboutToHideConnections[menu]

Copilot uses AI. Check for mistakes.
}
}
menu.open()
menu.aboutToHide.connect(() => { activeMenu = null })

// Create and store the handler for this specific menu
let handler = function() {
activeMenu = null
}
aboutToHideConnections[menu] = handler
menu.aboutToHide.connect(handler)

activeMenu = menu
}
function closeMenu(menu: Menu) {
Expand Down
34 changes: 25 additions & 9 deletions panels/dock/package/main.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Window {
(Screen.width - dockLeftPart.implicitWidth - dockRightPart.implicitWidth)
property int dockRemainingSpaceForCenter: useColumnLayout ?
(Screen.height / 1.8 - dockRightPart.implicitHeight) :
(Screen.width / 1.8 - dockRightPart.implicitWidth)
(Screen.width / 1.8 - dockRightPart.implicitWidth)
property int dockPartSpacing: gridLayout.columnSpacing
// TODO
signal dockCenterPartPosChanged()
Expand All @@ -42,6 +42,9 @@ Window {

property real dockItemIconSize: dockItemMaxSize * 9 / 14

// LauncherController wrapper for optional launchpad support
property var launcherControllerWrapper: null

// NOTE: -1 means not set its size, follow the platform size
width: Panel.position === Dock.Top || Panel.position === Dock.Bottom ? -1 : dockSize
height: Panel.position === Dock.Left || Panel.position === Dock.Right ? -1 : dockSize
Expand All @@ -65,7 +68,7 @@ Window {
D.DWindow.windowRadius: 0
//TODO：由于windoweffect处理有BUG，导致动画结束后一致保持无阴影，无borderwidth状态。 无法恢复到最初的阴影和边框
//D.DWindow.windowEffect: hideShowAnimation.running ? D.PlatformHandle.EffectNoShadow | D.PlatformHandle.EffectNoBorder : 0

// 目前直接处理shadowColor(透明和默认值的切换)和borderWidth(0和1的切换)，来控制阴影和边框
// 默认阴影透明度是 60%，见： https://github.com/linuxdeepin/qt5platform-plugins/blob/master/xcb/dframewindow.h#L122
D.DWindow.shadowColor: hideShowAnimation.running ? Qt.rgba(0, 0, 0, 0) : Qt.rgba(0, 0, 0, 0.6)
Expand Down Expand Up @@ -119,13 +122,13 @@ Window {
Connections {
target: dockTransform
enabled: Qt.platform.pluginName === "xcb" && hideShowAnimation.running

function onXChanged() {
if (dock.useColumnLayout) {
Panel.notifyDockPositionChanged(dockTransform.x, 0)
}
}

function onYChanged() {
if (!dock.useColumnLayout) {
Panel.notifyDockPositionChanged(0, dockTransform.y)
Expand All @@ -136,13 +139,13 @@ Window {
Connections {
target: dock
enabled: Qt.platform.pluginName !== "xcb" && hideShowAnimation.running

function onWidthChanged() {
if (dock.useColumnLayout) {
Panel.notifyDockPositionChanged(dock.width, 0)
}
}

function onHeightChanged() {
if (!dock.useColumnLayout) {
Panel.notifyDockPositionChanged(0, dock.height)
Expand Down Expand Up @@ -458,18 +461,18 @@ Window {
}
}

Item {
Item {
id: dockCenterPart
property var taskmanagerRootObject: {
let applet = DS.applet("org.deepin.ds.dock.taskmanager")
return applet ? applet.rootObject : null
}

readonly property real taskmanagerImplicitWidth: taskmanagerRootObject ? taskmanagerRootObject.implicitWidth : 0
readonly property real taskmanagerImplicitHeight: taskmanagerRootObject ? taskmanagerRootObject.implicitHeight : 0
readonly property real taskmanagerAppContainerWidth: taskmanagerRootObject ? taskmanagerRootObject.appContainerWidth : 0
readonly property real taskmanagerAppContainerHeight: taskmanagerRootObject ? taskmanagerRootObject.appContainerHeight : 0

implicitWidth: centerLoader.implicitWidth - taskmanagerImplicitWidth + taskmanagerAppContainerWidth
implicitHeight: centerLoader.implicitHeight - taskmanagerImplicitHeight + taskmanagerAppContainerHeight
onXChanged: dockCenterPartPosChanged()
Expand Down Expand Up @@ -685,6 +688,10 @@ Window {
DS.closeChildrenWindows(popup)
if (popup && popup.visible)
popup.close()
// Close dde-launchpad if available
if (launcherControllerWrapper) {
launcherControllerWrapper.hide()
}
}
function onDockScreenChanged() {
// Close all popups when dock moves to another screen
Expand Down Expand Up @@ -744,5 +751,14 @@ Window {
dock.itemIconSizeBase = dock.dockItemMaxSize
dock.visible = Panel.hideState !== Dock.Hide
changeDragAreaAnchor()

// Try to create LauncherController wrapper for optional launchpad support
try {
launcherControllerWrapper = Qt.createQmlObject(
Copy link
Contributor

@18202781743 18202781743 Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这种不应该在dde-shell中依赖launchpad，

'import QtQuick 2.15; import org.deepin.launchpad 1.0; QtObject { function hide() { LauncherController.visible = false } }',
dock, "LauncherControllerWrapper")
Comment on lines +758 to +759
Copy link
Member

@BLumia BLumia Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

啊？这里的目的仅仅是为了获取LauncherController以便你隐藏launchpad是么？

} catch (e) {
// Launchpad module not available, launcherControllerWrapper remains null
Comment on lines +757 to +761
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline QML string creates a tight coupling and makes the code harder to maintain. Consider extracting this to a separate QML file (e.g., LauncherControllerWrapper.qml) that can be loaded conditionally using a Loader or Qt.createComponent, which would improve readability and make the wrapper logic easier to test and modify.

Suggested change
launcherControllerWrapper = Qt.createQmlObject(
'import QtQuick 2.15; import org.deepin.launchpad 1.0; QtObject { function hide() { LauncherController.visible = false } }',
dock, "LauncherControllerWrapper")
} catch (e) {
// Launchpad module not available, launcherControllerWrapper remains null
var launcherComponent = Qt.createComponent("LauncherControllerWrapper.qml");
if (launcherComponent.status === Component.Ready) {
launcherControllerWrapper = launcherComponent.createObject(dock);
}
} catch (e) {
// Launchpad module or wrapper component not available, launcherControllerWrapper remains null

Copilot uses AI. Check for mistakes.
}
}
}