refactor: move XdgActivation to pluginitem and simplify dock plugins#468
Merged
Merged
Conversation
1. Remove XdgActivation usage from all dock plugins (airplane, bluetooth, brightness, datetime, dnd, eye-comfort, keyboard, notification, power, shutdown, sound, wireless-casting) 2. Move XdgActivation request to PluginItem's invokeMenuItem and executeCommand methods 3. Standardize launching control-center with dde-am command without XdgActivation token 4. Set XDG_ACTIVATION_TOKEN environment variable in pluginitem before calling plugin's invokedMenuItem 5. Simplify command execution by using QProcess with environment variable Log: Refactored XdgActivation handling for dock plugin system Influence: 1. Verify all dock plugin settings buttons still open control-center correctly 2. Test that XDG_ACTIVATION_TOKEN is properly set when invoking plugin menu items 3. Verify command execution with XdgActivation token works 4. Test that plugins without settings (e.g., airplane mode toggle) still work 5. Verify no regressions in dock plugin functionality 6. Test keyboard layout add layout action refactor: 将XdgActivation移到pluginitem并简化dock插件 1. 从所有dock插件中移除XdgActivation使用（飞行模式、蓝牙、亮度、日期时 间、勿扰、护眼、键盘布局、通知、电源、关机、声音、投屏） 2. 将XdgActivation请求移动到PluginItem的invokeMenuItem和executeCommand方 法中 3. 标准化使用dde-am命令启动控制中心，不再传递XdgActivation令牌 4. 在调用插件invokedMenuItem之前，在pluginitem中设置XDG_ACTIVATION_TOKEN 环境变量 5. 通过使用带环境变量的QProcess简化命令执行 Log: 重构了dock插件系统的XdgActivation处理 Influence: 1. 验证所有dock插件设置按钮仍然正确打开控制中心 2. 测试调用插件菜单项时是否正确设置XDG_ACTIVATION_TOKEN 3. 验证带XdgActivation令牌的命令执行是否正常 4. 测试无设置的插件（如飞行模式切换）是否仍然正常工作 5. 验证dock插件功能没有回归问题 6. 测试键盘布局添加布局的操作
18202781743
requested review from
BLumia,
fly602,
mhduiy,
robertkill,
wjyrich and
yixinshark
Reviewer's GuideCentralizes XdgActivation handling in PluginItem and removes per-plugin logic while standardizing how dock plugins launch control-center via dde-am and execute commands with XDG_ACTIVATION_TOKEN set via QProcess. Sequence diagram for PluginItem invokeMenuItem central XdgActivation handlingsequenceDiagram
participant PluginItem
participant XdgActivation as tray_XdgActivation
participant Environment
participant PluginsItemInterface
PluginItem->>XdgActivation: requestToken()
XdgActivation-->>PluginItem: tokenReady(token)
alt token not empty
PluginItem->>Environment: qputenv(XDG_ACTIVATION_TOKEN, token)
PluginItem->>PluginsItemInterface: invokedMenuItem(m_itemKey, menuId, checked)
PluginItem->>Environment: qunsetenv(XDG_ACTIVATION_TOKEN)
else token empty
PluginItem->>PluginsItemInterface: invokedMenuItem(m_itemKey, menuId, checked)
end
PluginItem->>XdgActivation: deleteLater()
Sequence diagram for PluginItem executeCommand with XDG_ACTIVATION_TOKENsequenceDiagram
participant PluginItem
participant XdgActivation as tray_XdgActivation
participant QProcess
PluginItem->>XdgActivation: requestToken()
XdgActivation-->>PluginItem: tokenReady(token)
PluginItem->>QProcess: QProcess()
PluginItem->>QProcess: setProcessEnvironment(systemEnvironment + XDG_ACTIVATION_TOKEN)
PluginItem->>QProcess: startDetached(program, commandList)
PluginItem->>XdgActivation: deleteLater()
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 PluginItem::executeCommand, the QProcess environment you set is never used because you call process.startDetached(program, commandList), which uses the static overload and ignores the instance’s environment; either use the parameterless startDetached() after configuring program/arguments or use a static overload that accepts an environment.
- The use of qputenv/qunsetenv around XDG_ACTIVATION_TOKEN in PluginItem::invokeMenuItem introduces a global process-wide change that could race or conflict if multiple PluginItem instances invoke menu items concurrently; consider a more localized way to pass the token (e.g., through QProcessEnvironment only for spawned processes) instead of mutating the global environment.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In PluginItem::executeCommand, the QProcess environment you set is never used because you call process.startDetached(program, commandList), which uses the static overload and ignores the instance’s environment; either use the parameterless startDetached() after configuring program/arguments or use a static overload that accepts an environment.
- The use of qputenv/qunsetenv around XDG_ACTIVATION_TOKEN in PluginItem::invokeMenuItem introduces a global process-wide change that could race or conflict if multiple PluginItem instances invoke menu items concurrently; consider a more localized way to pass the token (e.g., through QProcessEnvironment only for spawned processes) instead of mutating the global environment.
## Individual Comments
### Comment 1
<location path="src/loader/pluginitem.cpp" line_range="48-55" />
<code_context>
if (menuId == SHIFT) {
AMC_PTR->toggle();
} else if (menuId == SETTINGS) {
- auto *activation = new tray::XdgActivation(this);
- connect(activation, &tray::XdgActivation::tokenReady, this, [activation](const QString &token) {
- QStringList args {"--by-user", "org.deepin.dde.control-center"};
- if (!token.isEmpty())
- args << "-e" << "XDG_ACTIVATION_TOKEN=" + token;
- args << "--" << "-p" << "network/airplaneMode";
- QProcess::startDetached("dde-am", args);
- activation->deleteLater();
- }, Qt::SingleShotConnection);
- activation->requestToken();
</code_context>
<issue_to_address>
**issue (bug_risk):** Using qputenv/qunsetenv for XDG_ACTIVATION_TOKEN is process-global and can race with other code.
This approach depends on `invokedMenuItem` being strictly synchronous: any async work it triggers (queued signals, timers, other threads) could observe no token or the wrong token. While the token is set, any unrelated child processes started in this process will also inherit it. Please either scope the token to specific process launches (e.g., via a per-process `QProcessEnvironment` instead of mutating the global environment) or clearly guarantee/document that `invokedMenuItem` is synchronous and does not start background work that launches processes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
mhduiy
approved these changes
Jun 12, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy 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 |
|
@18202781743: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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
3 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.
bluetooth, brightness, datetime, dnd, eye-comfort, keyboard,
notification, power, shutdown, sound, wireless-casting)
executeCommand methods
XdgActivation token
calling plugin's invokedMenuItem
variable
Log: Refactored XdgActivation handling for dock plugin system
Influence:
correctly
menu items
work
refactor: 将XdgActivation移到pluginitem并简化dock插件
间、勿扰、护眼、键盘布局、通知、电源、关机、声音、投屏）
法中
环境变量
Log: 重构了dock插件系统的XdgActivation处理
Influence:
Summary by Sourcery
Centralize XdgActivation handling in the dock plugin framework and standardize how dock plugins launch the control center and external commands.
Enhancements: