Skip to content
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.

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

fix: [pluginmanager] can`t switch navigation after switched to plugin… #887

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

LiHua000
Copy link
Contributor

…manager by menu action

Log: as title

@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • PluginManagerModule::initialize函数中,创建了QAction对象menuAction,但未看到对应的destroyed信号连接,可能会导致内存泄漏。
  • QObject::connect使用了Lambda表达式,但没有捕获任何外部变量,这可能会导致悬垂指针的风险。
  • uiController->addAction的第二个参数从actionOptionsImpl变更为new AbstractAction(menuAction),需要确认AbstractAction类的行为是否正确处理了QAction对象。
  • 新增的QAction图标通过QIcon::fromTheme("plugins-navigation")获取,但没有检查图标是否有效。
  • 连接menuActiontriggered信号到Lambda表达式时,Lambda表达式捕获了this,这意味着menuAction对象被隐式传递给Lambda,这可能会导致this在上下文中不正确。

是否建议立即修改:

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Kakueeen, LiHua000
Once this PR has been reviewed and has the lgtm label, please assign eric2023 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-mozart deepin-mozart merged commit ca3dd4b into linuxdeepin:master Aug 16, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants