Repository navigation
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Resurgamz 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 GuidePDF printing now performs the required Sequence diagram for asynchronous PDF print preparationsequenceDiagram
actor User
participant DocSheet
participant WaitTip as DFloatingMessage
participant Process as QProcess
participant Preview as DPrintPreviewDialog
User->>DocSheet: onPopPrintDialog()
DocSheet->>WaitTip: showPrintWaitTip()
DocSheet->>Process: start(pdftocairo)
Process-->>DocSheet: finished(exitCode, exitStatus)
DocSheet->>DocSheet: isConvertedPdfValid()
alt conversion succeeds and output is valid
DocSheet->>WaitTip: closePrintWaitTip()
DocSheet->>Preview: showPrintPreviewWithPath()
Preview-->>User: open()
else failure, timeout, or invalid output
DocSheet->>WaitTip: closePrintWaitTip()
DocSheet->>User: showPrintFailedTip()
end
Flow diagram for cached PDF print conversionflowchart TD
A[PDF print requested] --> B{Conversion already running?}
B -- Yes --> C[Ignore repeated request]
B -- No --> D{Valid cached PDF exists?}
D -- Yes --> E[showPrintPreviewWithPath]
D -- No --> F[showPrintWaitTip]
F --> G[startPdfPrintConversion]
G --> H{Conversion succeeds and output is valid?}
H -- Yes --> I[Cache converted PDF]
I --> E
H -- No --> J[Abort printing]
J --> K[showPrintFailedTip]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
TAG Bot TAG: 6.5.67 |
![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 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="reader/uiframe/DocSheet.cpp" line_range="2122-2123" />
<code_context>
+ if (m_printConvertTimer)
+ m_printConvertTimer->stop();
+ if (m_printConvertProcess && m_printConvertProcess->state() != QProcess::NotRunning)
+ m_printConvertProcess->kill();
+
// 排空引用本 sheet 的所有待处理渲染任务(仅优化,避免为将死文档做无用渲染);
</code_context>
<issue_to_address>
**Retries fail after conversion timeouts**
When the user retries printing before the killed process has finished, the timeout handler calls `finishPdfPrintConversion(false)`, clearing the in-progress guard while `m_printConvertProcess` is still running; the retry’s `start()` does not launch a new conversion, and the old process’s callback can finish the retry as a failure or use its output.
Keep retries from starting until the timed-out process has stopped, and prevent its callbacks from completing a later request.
Also at `reader/uiframe/DocSheet.cpp:2098-2105`, `reader/uiframe/DocSheet.cpp:2129-2130`.
</issue_to_address>
### Comment 2
<location path="tests/uiframe/ut_docsheet.cpp" line_range="1729-1732" />
<code_context>
+TEST_F(TestDocSheet, UT_DocSheet_finishPdfPrintConversion_failure)
+{
+ // 处理失败：退出等待状态并给出失败提示，且不进行后续打印
+ m_tester->m_printConvertInProgress = true;
+ m_tester->finishPdfPrintConversion(false);
+ EXPECT_FALSE(m_tester->m_printConvertInProgress);
+}
+
TEST_F(TestDocSheet, UT_DocSheet_onPrintRequested_singleArg)
</code_context>
<issue_to_address>
**Failure behavior goes untested**
When the failure branch stops showing the error tip or incorrectly opens a preview, `UT_DocSheet_finishPdfPrintConversion_failure` checks only that the in-progress flag is cleared, so it still passes without failure feedback or with a preview opened.
Assert that the failure tip is shown and that no print preview is opened.
</issue_to_address>
Comment on lines
+2122
to
+2123
| m_printConvertProcess->kill(); | ||
| finishPdfPrintConversion(false); |
There was a problem hiding this comment.
🟡 Medium · Retries fail after conversion timeouts
When the user retries printing before the killed process has finished, the timeout handler calls finishPdfPrintConversion(false), clearing the in-progress guard while m_printConvertProcess is still running; the retry’s start() does not launch a new conversion, and the old process’s callback can finish the retry as a failure or use its output.
Keep retries from starting until the timed-out process has stopped, and prevent its callbacks from completing a later request.
Also at reader/uiframe/DocSheet.cpp:2098-2105, reader/uiframe/DocSheet.cpp:2129-2130.
Prompt for AI agents
In `reader/uiframe/DocSheet.cpp` at lines 2122-2123:
**Retries fail after conversion timeouts**
When the user retries printing before the killed process has finished, the timeout handler calls `finishPdfPrintConversion(false)`, clearing the in-progress guard while `m_printConvertProcess` is still running; the retry’s `start()` does not launch a new conversion, and the old process’s callback can finish the retry as a failure or use its output.
Keep retries from starting until the timed-out process has stopped, and prevent its callbacks from completing a later request.
Also at `reader/uiframe/DocSheet.cpp:2098-2105`, `reader/uiframe/DocSheet.cpp:2129-2130`.
Comment on lines
+1729
to
+1732
| m_tester->m_printConvertInProgress = true; | ||
| m_tester->finishPdfPrintConversion(false); | ||
| EXPECT_FALSE(m_tester->m_printConvertInProgress); | ||
| } |
There was a problem hiding this comment.
🟡 Medium · Failure behavior goes untested
When the failure branch stops showing the error tip or incorrectly opens a preview, UT_DocSheet_finishPdfPrintConversion_failure checks only that the in-progress flag is cleared, so it still passes without failure feedback or with a preview opened.
Assert that the failure tip is shown and that no print preview is opened.
Prompt for AI agents
In `tests/uiframe/ut_docsheet.cpp` at lines 1729-1732:
**Failure behavior goes untested**
When the failure branch stops showing the error tip or incorrectly opens a preview, `UT_DocSheet_finishPdfPrintConversion_failure` checks only that the in-progress flag is cleared, so it still passes without failure feedback or with a preview opened.
Assert that the failure tip is shown and that no print preview is opened.
Resurgamz
force-pushed
the
agent/pms-bug-bot/7de3b1406a7e
branch
3 times, most recently
from
October 19, 2026 09:38
9f8b8d8 to
a5e6ec7
Compare
Resurgamz
force-pushed
the
agent/pms-bug-bot/7de3b1406a7e
branch
from
October 19, 2026 10:01
a5e6ec7 to
f5df335
Compare
pdftocairo was executed synchronously on the UI thread before the print preview was shown, so large PDFs blocked the print dialog from popping up. Now a persistent lightweight tip is shown immediately, the conversion runs in a background process, and the preview opens once it finishes. If the conversion fails or pdftocairo is unavailable, it falls back to the original PDF so printing still proceeds (same behaviour as before). The conversion is performed on every print. The conversion is still performed for every PDF (not gated by the Linearized flag) so that bug-348017 (non-linearized PDFs with non-embedded CJK fonts printing blank/garbled) is not regressed. Log: 打印 PDF 时异步执行 pdftocairo 转化并先显示等待提示，避免大文件阻塞打印界面 Bug: https://pms.uniontech.com/bug-view-379067.html
Resurgamz
force-pushed
the
agent/pms-bug-bot/7de3b1406a7e
branch
from
October 20, 2026 13:16
f5df335 to
b4e123b
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ❌评价： 存在逻辑缺陷 ❌ 不通过 潜在问题： 建议： ['printTipParent 直接返回 this->window()，避免遍历全部顶层窗口并消除多窗口错位问题', '析构 kill() 前显式 disconnect(m_printConvertProcess/timer, nullptr, this, nullptr)，作为防御性加固'] 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 潜在问题： 建议： ['等待提示使用加载中/旋转图标（如 view-refresh 或 DSpinner）替代 dr_ok', '将 qCDebug 日志移入各退出路径，或直接移除', '失败回退时向用户展示简短警告（如另起一条 DFloatingMessage 或状态栏提示）'] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： ['使用 QStringLiteral("dr_ok") 替代运行期字符串拼接'] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： [] 💡 改进建议代码示例// DocSheet.cpp —— 修正 printTipParent 窗口定位，并优化图标与日志
QWidget *DocSheet::printTipParent() const
{
// 直接取当前 sheet 所在的顶层窗口，避免多窗口时提示显示到错误窗口
QWidget *parent = this->window();
if (!parent)
parent = qApp->activeWindow();
if (!parent)
parent = const_cast<DocSheet *>(this);
return parent;
}
void DocSheet::showPrintWaitTip()
{
if (m_printWaitTip)
return;
QWidget *parent = printTipParent();
if (!parent)
return;
DFloatingMessage *tip = new DFloatingMessage(DFloatingMessage::ResidentType);
tip->setAttribute(Qt::WA_DeleteOnClose);
// 使用加载中图标，语义与“准备中请等待”一致
tip->setIcon(QIcon::fromTheme(QStringLiteral("view-refresh")));
tip->setMessage(tr("Preparing the content for printing. This may take a while. Please wait…"));
if (DDialogCloseButton *closeBtn = tip->findChild<DDialogCloseButton *>())
closeBtn->hide();
m_printWaitTip = tip;
DMessageManager::instance()->sendMessage(parent, tip);
qCDebug(appLog) << "print wait tip shown";
}本报告由 AI 代码审查工具自动生成 |
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.
Problem
Clicking Print /
Ctrl+Pon a PDF ranpdftocairosynchronously on the UI thread inDocSheet::onPopPrintDialog()beforepreview->open(). For PDFs with many pages the conversion can take a long time, so the print preview dialog could not pop up and the whole UI froze. A prior fix even raised the timeout to 5 minutes, which made the freeze worse.Change
pdftocairoconversion asynchronously in a backgroundQProcess; the UI thread is never blocked.DFloatingMessage/DMessageManagerstyle (close button hidden), and open the print preview once the conversion finishes.pdftocairois unavailable, fall back to the original PDF and continue printing (same behaviour as before this change).Translations for the new string are submitted separately in #422.
Regression constraint
The conversion is intentionally not gated by the
Linearizedflag. The bug-348017 reproducers are non-linearized PDFs with non-embedded CJK fonts; skipping the conversion for them would reintroduce blank/garbled printing. Every PDF is still converted exactly as before — only the timing/threading changed.Testing
cmake --buildofdeepin-readerpasses (Qt5/DTK5).Related
Linearized