Skip to content

fix(multi-select): disable save voice button when no voice in selecte…#352

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/disable-save-voice-when-no-voice
Jun 1, 2026
Merged

fix(multi-select): disable save voice button when no voice in selecte…#352
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/disable-save-voice-when-no-voice

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

…d notes

Call checkNoteVoice/checkNoteText in onSelectSizeChanged handler when entering multi-select mode, matching existing context menu pattern.

多选笔记时在onSelectSizeChanged中调用checkNoteVoice/checkNoteText, 与右键菜单逻辑保持一致,确保无语音笔记被选中时保存语音按钮置灰。

Log: 多选无语音笔记时保存语音按钮置灰
PMS: BUG-339519
Influence: 多选笔记时正确判断是否有语音,无语音则保存语音按钮置灰;同时修复checkNoteVoice空指针问题。

…d notes

Call checkNoteVoice/checkNoteText in onSelectSizeChanged handler when
entering multi-select mode, matching existing context menu pattern.

多选笔记时在onSelectSizeChanged中调用checkNoteVoice/checkNoteText,
与右键菜单逻辑保持一致,确保无语音笔记被选中时保存语音按钮置灰。

Log: 多选无语音笔记时保存语音按钮置灰
PMS: BUG-339519
Influence: 多选笔记时正确判断是否有语音,无语音则保存语音按钮置灰;同时修复checkNoteVoice空指针问题。
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "src/common/VNoteMainManager.cpp": [
        {
            "line": "            url = \"https://www.deepin.org/zh/agreement/privacy/\";",
            "line_number": 1187,
            "rule": "S35",
            "reason": "Url link | 20e2eab189"
        },
        {
            "line": "            url = \"https://www.uniontech.com/agreement/privacy-cn\";",
            "line_number": 1189,
            "rule": "S35",
            "reason": "Url link | 4850a00dd7"
        },
        {
            "line": "            url = \"https://www.deepin.org/en/agreement/privacy/\";",
            "line_number": 1193,
            "rule": "S35",
            "reason": "Url link | 38d42f63bf"
        },
        {
            "line": "            url = \"https://www.uniontech.com/agreement/privacy-en\";",
            "line_number": 1195,
            "rule": "S35",
            "reason": "Url link | f82409d3b5"
        }
    ]
}

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。这次修改主要涉及空指针安全修复多选状态下的批量检查逻辑

总体来说,这次修改修复了一个严重的崩溃隐患,并增加了多选时的业务逻辑。以下是我对各个维度的详细审查意见和改进建议:

1. 语法与逻辑

C++ 部分 (VNoteMainManager.cpp)

  • 评价:修改非常正确且必要。getNoteById(noteIndex) 在找不到对应ID时极有可能返回 nullptr,原代码直接调用 item->haveVoice() 会导致空指针解引用,引发程序崩溃。加入 item && 的短路逻辑是标准的防御性编程。

QML 部分 (ItemListView.qml)

  • 潜在逻辑缺陷:当 selectSize> 1 变回 1(即从多选恢复到单选)或 0(取消选择)时,没有对应的逻辑去重置或更新状态。如果业务逻辑要求在单选时也进行检查,或者需要清除多选时的检查状态,当前代码无法覆盖。
  • 建议:确认业务需求。如果单选时也需要检查,应将 if (selectSize > 1) 改为 if (selectSize >= 1);如果需要在取消多选时重置状态,需补充 else 分支。

2. 代码质量

QML 部分 (ItemListView.qml)

  • 问题:在 onSelectSizeChanged 信号处理器中直接编写了较长的循环和业务调用逻辑,这使得代码显得臃肿,不利于阅读和复用。
  • 建议:将构建 list 和调用 VNoteMainManager 的逻辑提取为一个独立的函数,例如 checkSelectedNotes(),然后在信号处理器中调用该函数。

3. 代码性能

QML 部分 (ItemListView.qml)

  • 问题 1:隐式类型转换与内存分配:代码使用 var list = []; 声明数组。在QML/JavaScript引擎中,var 会被推断为 JS 的动态数组,频繁的 push 操作可能导致内存重新分配。由于这里是将数据传递给 C++ 端的 QVariantList,直接在 C++ 端处理或使用更明确的类型可能更高效。
  • 问题 2:不必要的重复遍历:代码中先遍历了一次 selectedNoteItem 构建 list,然后分别调用了 checkNoteVoice(list)checkNoteText(list)。这意味着 C++ 端需要对同一个 list 进行两次遍历。
  • 建议
    • 方案A(推荐):在 C++ 端提供一个合并后的接口,例如 checkNoteVoiceAndText(const QVariantList &index),一次遍历同时检查语音和文本,减少跨语言调用开销和遍历次数。
    • 方案B:如果 C++ 接口不宜修改,可以在 QML 中提前分配数组大小(虽然 JS 数组无此方法,但可使用 new Array(length) 并按索引赋值,性能略优于 push)。

4. 代码安全

C++ 部分 (VNoteMainManager.cpp)

  • 评价:此次修改消除了空指针解引用的安全隐患,非常棒。
  • 延伸建议:注意 id.toInt() 的安全性。如果传入的 QVariant 中包含非整型数据(如字符串),toInt() 会返回 0 或转换失败,这可能导致 getNoteById(0) 返回意外结果。建议在调用 toInt() 时使用 bool ok 参数进行类型严格校验,或者确保上游数据绝对可靠。

QML 部分 (ItemListView.qml)

  • 问题:数组越界风险itemModel.get(selectedNoteItem[i]) 存在潜在的越界风险。如果在选择状态改变和处理逻辑执行之间,数据源(itemModel)发生了变化(例如后台删除了某个 note),selectedNoteItem[i] 对应的索引可能已经无效,导致 get() 返回 undefined,随后访问 .noteId 将引发 QML 运行时错误。
  • 建议:增加安全访问检查。

综合改进后的代码建议

基于以上分析,我为你提供优化后的代码实现:

C++ 部分 (进一步加固)

void VNoteMainManager::checkNoteVoice(const QVariantList &index)
{
    bool hasVoice = false;
    foreach (auto id, index) {
        bool ok = false;
        int noteIndex = id.toInt(&ok); // 增加类型转换安全校验
        if (!ok) continue;             // 如果转换失败,跳过该项

        VNoteItem *item = getNoteById(noteIndex);
        if (item && item->haveVoice()) { // 保留了你的空指针修复
            hasVoice = true;
            break;
        }
    }
    // ... 后续逻辑
}

QML 部分 (提升性能、安全性与可读性)

// 提取为独立函数,提升代码可读性与复用性
function checkSelectedNotes() {
    if (selectedNoteItem.length <= 1) {
        // 可选:处理单选或无选择时的逻辑重置
        return; 
    }

    // 提前分配数组大小,比 push 略微提升性能
    var list = new Array(selectedNoteItem.length);
    for (var i = 0; i < selectedNoteItem.length; i++) {
        var modelItem = itemModel.get(selectedNoteItem[i]);
        // 防御性编程:防止 model 发生变动导致越界或访问 undefined
        if (modelItem && modelItem.noteId !== undefined) {
            list[i] = modelItem.noteId;
        } else {
            list[i] = -1; // 填充无效ID,C++端应能处理无效ID
        }
    }

    // 性能优化建议:如果C++端能合并这两个接口为 checkNoteVoiceAndText(list) 会更好
    VNoteMainManager.checkNoteVoice(list);
    VNoteMainManager.checkNoteText(list);
}

onSelectSizeChanged: {
    mulChoices(selectSize);
    checkSelectedNotes(); // 调用封装好的逻辑
}

希望这些审查意见和代码改进能对你有所帮助!如果有任何疑问,欢迎随时提问。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pengfeixx

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

Details 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

@pengfeixx
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit b002e59 into linuxdeepin:develop/snipe Jun 1, 2026
18 checks passed
@pengfeixx pengfeixx deleted the fix/disable-save-voice-when-no-voice branch June 1, 2026 05:06
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.

3 participants