fix: filter unsupported dock applets in DBus proxy#1563
Open
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: filter unsupported dock applets in DBus proxy#1563wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
1. Added isSupported property to DAppletDock class to mark applet compatibility 2. Modified dock DBus proxy to skip unsupported applets when updating plugin visibility and listing plugins 3. Enhanced MultiTaskView to dynamically update supported status based on window compositor availability 4. Fixed visible property calculation to consider both m_visible and m_isSupported flags 5. Added proper signal emission when supported status changes affect effective visibility Log: Fixed dock applet visibility issues when compositor is unavailable Influence: 1. Test dock applet visibility when switching between composited and non-composited modes 2. Verify DBus plugin list excludes unsupported applets 3. Check that MultiTaskView correctly appears/disappears based on compositor support 4. Test that visibleChanged signal fires appropriately when supported status changes 5. Verify dock settings UI correctly reflects available applets fix: 在DBus代理中过滤不支持的停靠小程序 1. 在DAppletDock类中添加isSupported属性以标记小程序兼容性 2. 修改停靠DBus代理，在更新插件可见性和列出插件时跳过不支持的小程序 3. 增强MultiTaskView以根据窗口合成器可用性动态更新支持状态 4. 修复visible属性计算，同时考虑m_visible和m_isSupported标志 5. 当支持状态变化影响有效可见性时添加适当的信号发射 Log: 修复合成器不可用时停靠小程序可见性问题 Influence: 1. 测试在合成和非合成模式间切换时停靠小程序的可见性 2. 验证DBus插件列表排除不支持的小程序 3. 检查MultiTaskView是否根据合成器支持正确显示/隐藏 4. 测试当支持状态变化时visibleChanged信号是否适当触发 5. 验证停靠设置UI正确反映可用的小程序
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideAdds an explicit supported state to dock applets, wires MultiTaskView’s support to compositor/KWin effect availability, and updates the DBus proxy to ignore unsupported applets for visibility and plugin listing, ensuring visibility and signals reflect both user choice and platform support. Sequence diagram for compositor changes affecting MultiTaskView visibilitysequenceDiagram
participant DWindowManagerHelper
participant MultiTaskView
participant DAppletDock as BaseDockApplet
DWindowManagerHelper->>MultiTaskView: hasCompositeChanged(newHasComposite)
activate MultiTaskView
MultiTaskView-->>MultiTaskView: Q_EMIT visibleChanged()
MultiTaskView->>MultiTaskView: setSupported(m_kWinEffect && hasComposite())
alt supported state unchanged
MultiTaskView-->>MultiTaskView: return (no signal change)
else supported state changed
MultiTaskView->>BaseDockApplet: setSupported(bool supported)
activate BaseDockApplet
BaseDockApplet-->>BaseDockApplet: oldEffectiveVisible = visible()
BaseDockApplet-->>BaseDockApplet: m_isSupported = supported
BaseDockApplet-->>BaseDockApplet: Q_EMIT supportedChanged()
alt effective visibility changed
BaseDockApplet-->>BaseDockApplet: Q_EMIT visibleChanged()
end
deactivate BaseDockApplet
end
deactivate MultiTaskView
Sequence diagram for DockDBusProxy filtering unsupported dock appletssequenceDiagram
participant Client
participant DockDBusProxy
participant DockApplet1
participant DockApplet2
Client->>DockDBusProxy: updateDockPluginsVisible(pluginsVisible)
activate DockDBusProxy
loop for each dockApplet in dockApplets(parent)
alt dockApplet is unsupported
DockDBusProxy->>DockApplet1: isSupported()
DockApplet1-->>DockDBusProxy: false
DockDBusProxy-->>DockDBusProxy: continue (skip applet)
else dockApplet is supported
DockDBusProxy->>DockApplet2: isSupported()
DockApplet2-->>DockDBusProxy: true
DockDBusProxy->>DockApplet2: dockItemInfo()
DockApplet2-->>DockDBusProxy: DockItemInfo
DockDBusProxy-->>DockDBusProxy: apply pluginsVisible[itemKey]
end
end
deactivate DockDBusProxy
Client->>DockDBusProxy: plugins()
activate DockDBusProxy
loop for each dockApplet in dockApplets(parent)
alt dockApplet is unsupported
DockDBusProxy->>DockApplet1: isSupported()
DockApplet1-->>DockDBusProxy: false
DockDBusProxy-->>DockDBusProxy: skip appending
else dockApplet is supported
DockDBusProxy->>DockApplet2: isSupported()
DockApplet2-->>DockDBusProxy: true
DockDBusProxy->>DockApplet2: dockItemInfo()
DockApplet2-->>DockDBusProxy: DockItemInfo
DockDBusProxy-->>DockDBusProxy: iteminfos.append(info)
end
end
DockDBusProxy-->>Client: DockItemInfos (supported applets only)
deactivate DockDBusProxy
Class diagram for dock applet support and visibility modelclassDiagram
class DAppletDockPrivate {
bool m_visible
bool m_isSupported
}
class DAppletDock {
+bool visible()
+void setVisible(bool visible)
+bool isSupported()
+void setSupported(bool supported)
+DockItemInfo dockItemInfo()
+void init()
<<signals>>
+void visibleChanged()
+void supportedChanged()
}
class MultiTaskView {
+MultiTaskView(QObject parent)
+bool init()
+DockItemInfo dockItemInfo()
-bool m_kWinEffect
-QScopedPointer multitaskview
}
class DockDBusProxy {
+QRect geometry()
+void updateDockPluginsVisible(QVariantMap pluginsVisible)
+DockItemInfos plugins()
}
class DWindowManagerHelper {
+static DWindowManagerHelper instance()
+bool hasComposite()
<<signals>>
+void hasCompositeChanged(bool hasComposite)
}
class DockItemInfo {
QString displayName
QString itemKey
QString settingKey
bool visible
QString dccIcon
}
DAppletDockPrivate <.. DAppletDock : uses
DAppletDock *-- DAppletDockPrivate : owns
MultiTaskView --|> DAppletDock
DockDBusProxy ..> DAppletDock : iterates_dockApplets
DWindowManagerHelper ..> MultiTaskView : notifies_compositor_change
DAppletDock ..> DockItemInfo : returns
MultiTaskView ..> DockItemInfo : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In MultiTaskView, the hasCompositeChanged and kWinEffect changed handlers both emit visibleChanged and then call setSupported, which can emit visibleChanged again; consider relying on setSupported’s effective-visibility detection to avoid redundant signal emissions.
- The condition
m_kWinEffect && DWindowManagerHelper::instance()->hasComposite()is duplicated in several places in MultiTaskView (constructor lambda, kWinEffect change, init); consider extracting a small helper (e.g.updateSupportFromCompositor()) or usingisSupported()consistently to keep the logic centralized and less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In MultiTaskView, the hasCompositeChanged and kWinEffect changed handlers both emit visibleChanged and then call setSupported, which can emit visibleChanged again; consider relying on setSupported’s effective-visibility detection to avoid redundant signal emissions.
- The condition `m_kWinEffect && DWindowManagerHelper::instance()->hasComposite()` is duplicated in several places in MultiTaskView (constructor lambda, kWinEffect change, init); consider extracting a small helper (e.g. `updateSupportFromCompositor()`) or using `isSupported()` consistently to keep the logic centralized and less error-prone.
## Individual Comments
### Comment 1
<location path="panels/dock/multitaskview/multitaskview.cpp" line_range="28-30" />
<code_context>
, m_iconName("deepin-multitasking-view")
{
- connect(DWindowManagerHelper::instance(), &DWindowManagerHelper::hasCompositeChanged, this, &MultiTaskView::visibleChanged);
+ connect(DWindowManagerHelper::instance(), &DWindowManagerHelper::hasCompositeChanged, this, [this]() {
+ Q_EMIT visibleChanged();
+ setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
+ });
auto platformName = QGuiApplication::platformName();
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid potentially emitting `visibleChanged` twice on compositor changes.
Because the lambda always emits `visibleChanged()`, and `setSupported()` can emit it again when the effective visibility changes, a single compositor change may trigger duplicate notifications. Either rely on `setSupported()` alone to emit `visibleChanged()` when needed, or emit `visibleChanged()` here only when `supported` actually changes (e.g. compute the new supported state, compare to `isSupported()`, then call `setSupported()` without the unconditional `Q_EMIT visibleChanged()`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+28
to
+30
| connect(DWindowManagerHelper::instance(), &DWindowManagerHelper::hasCompositeChanged, this, [this]() { | ||
| Q_EMIT visibleChanged(); | ||
| setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite()); |
There was a problem hiding this comment.
issue (bug_risk): Avoid potentially emitting visibleChanged twice on compositor changes.
Because the lambda always emits visibleChanged(), and setSupported() can emit it again when the effective visibility changes, a single compositor change may trigger duplicate notifications. Either rely on setSupported() alone to emit visibleChanged() when needed, or emit visibleChanged() here only when supported actually changes (e.g. compute the new supported state, compare to isSupported(), then call setSupported() without the unconditional Q_EMIT visibleChanged()).
deepin pr auto review这份代码diff主要实现了为Dock插件增加一个 1. 语法逻辑审查整体逻辑：代码逻辑基本正确。通过引入 潜在问题与改进：
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
总结建议
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: Fixed dock applet visibility issues when compositor is unavailable
Influence:
fix: 在DBus代理中过滤不支持的停靠小程序
Log: 修复合成器不可用时停靠小程序可见性问题
Influence:
Summary by Sourcery
Ensure dock applets respect both visibility and platform support, and exclude unsupported applets from DBus-exposed plugin state.
New Features:
Bug Fixes:
Enhancements: