From e752f54aa72c6248ce2c506b40b42eaf359829b9 Mon Sep 17 00:00:00 2001 From: Vadim Peretokin Date: Tue, 1 Sep 2026 10:55:01 +0200 Subject: [PATCH 1/2] fix: keep tracked links maintained when the model has no view The main console's model outlives the view built on it and keeps taking lines meanwhile, so gating the tracked-link adjustment on mpConsole stops maintaining it while the buffer is still being written to. Reach the manager through the model instead, and cover the announcement reset and the detached path that had no test. Assisted-by: Claude:claude-opus-5 --- src/TBuffer.cpp | 23 ++++-- src/TBuffer.h | 2 + src/THyperlinkVisibilityManager.h | 5 ++ test/functional_tests/TrackedLinkTrimTest.cpp | 71 +++++++++++++++++-- 4 files changed, 92 insertions(+), 9 deletions(-) diff --git a/src/TBuffer.cpp b/src/TBuffer.cpp index 99d25ac8081..f2618628564 100644 --- a/src/TBuffer.cpp +++ b/src/TBuffer.cpp @@ -5664,6 +5664,21 @@ bool TBuffer::replaceInLine(QPoint& P_begin, QPoint& P_end, const QString& with, return true; } +// The main console's model outlives the view built on it and keeps taking +// lines meanwhile, so going through mpConsole alone would stop maintaining the +// tracked links too early. Every other model is owned by its view and a +// clipboard slice has no model at all, which is where the nullptr comes from. +THyperlinkVisibilityManager* TBuffer::hyperlinkVisibilityManagerOrNull() +{ + if (mpConsole) { + return &mpConsole->getHyperlinkVisibilityManager(); + } + // mainConsoleModelOrNull() rather than mainConsoleModel() because this also + // runs from the TBuffer constructor, before Host holds the model. + TConsoleModel* pModel = mpHost.isNull() ? nullptr : mpHost->mainConsoleModelOrNull(); + return (pModel && this == &pModel->buffer) ? &pModel->mHyperlinkVisibilityManager : nullptr; +} + void TBuffer::clear() { // Clearing the display is not gagging: flush any deferred log text before @@ -5699,8 +5714,8 @@ void TBuffer::clear() mHyperlinkActive = false; // every line is about to go, so the tracked links and any announcement queued for them go with it - if (mpConsole) { // null while the buffer is being constructed - mpConsole->getHyperlinkVisibilityManager().clear(); + if (auto* pHyperlinkManager = hyperlinkVisibilityManagerOrNull()) { + pHyperlinkManager->clear(); } while (!buffer.empty()) { @@ -5928,8 +5943,8 @@ bool TBuffer::deleteLines(int from, int to) // Tracked OSC 8 hyperlinks are addressed by line number too, so they // shift with everything else - any whose line just went away are dropped - if (mpConsole) { - mpConsole->getHyperlinkVisibilityManager().adjustLineNumbers(from, delta); + if (auto* pHyperlinkManager = hyperlinkVisibilityManagerOrNull()) { + pHyperlinkManager->adjustLineNumbers(from, delta); } return true; } diff --git a/src/TBuffer.h b/src/TBuffer.h index 3642609dde9..f7b4cda0fd6 100644 --- a/src/TBuffer.h +++ b/src/TBuffer.h @@ -53,6 +53,7 @@ class QJsonObject; class QRegularExpression; class QTimer; class TConsole; +class THyperlinkVisibilityManager; class WrapInfo { @@ -431,6 +432,7 @@ class TBuffer bool mEchoingText = false; private: + THyperlinkVisibilityManager* hyperlinkVisibilityManagerOrNull(); inline QList getWrapInfo(const QString& lineText, bool isNewline, const int maxWidth, const int indent, const int hangingIndent); void shrinkBuffer(); void syncPreTriggerPassLine(int y); diff --git a/src/THyperlinkVisibilityManager.h b/src/THyperlinkVisibilityManager.h index ff03846b00b..6f4f36bc745 100644 --- a/src/THyperlinkVisibilityManager.h +++ b/src/THyperlinkVisibilityManager.h @@ -72,6 +72,11 @@ class THyperlinkVisibilityManager : public QObject { Q_OBJECT + // Reads the tracked line numbers and the queued announcement directly - the + // announcement is otherwise only observable through a screen reader, and a + // line number only through the side effect of revealing the link. + friend class TrackedLinkTrimTest; + public: explicit THyperlinkVisibilityManager(TConsoleModel& model); ~THyperlinkVisibilityManager() override; diff --git a/test/functional_tests/TrackedLinkTrimTest.cpp b/test/functional_tests/TrackedLinkTrimTest.cpp index 01a536928ce..7b334df7253 100644 --- a/test/functional_tests/TrackedLinkTrimTest.cpp +++ b/test/functional_tests/TrackedLinkTrimTest.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include "PortableModeTestHelper.h" @@ -128,6 +129,7 @@ private slots: QVERIFY2(!manager.trackedLinkIds().contains(5), "the link on the last removed line was kept, so entries accumulate"); QVERIFY2(manager.trackedLinkIds().contains(6), "the link on the first surviving line was dropped, so it can never reveal"); + QCOMPARE(manager.mTrackedLinks[6].lineNumber, 0); } // Concealing a link zeroes its characters' indices, so the scan that decides @@ -161,11 +163,10 @@ private slots: QCOMPARE(pConsole->getLinkStore().getHintsConst(linkId), QStringList{mLinkHint}); } - // A wholeline concealment deletes the link's own line, and deleteLines() now - // moves every link below it up one. The manager used to do that itself right - // afterwards, which would move them twice. Driven through onLinkClicked() - // because a click is the only thing that conceals without a delay, and there - // is no Lua entry point for one - see TBufferOSC_spec.lua, which says as much. + // A wholeline concealment deletes the link's own line, so every link below + // it moves up exactly one. Driven through onLinkClicked() because a click is + // the only thing that conceals without a delay, and there is no Lua entry + // point for one - see TBufferOSC_spec.lua, which says as much. void test_aWholelineConcealmentMovesTheLinksBelowItOnlyOnce() { auto* pConsole = mpHost->mpConsole.data(); @@ -231,6 +232,66 @@ private slots: QVERIFY2(pConsole->getLinkStore().getLinksConst(linkId).isEmpty(), "clearing the window left the link's command behind"); } + // A concealment queues a screen-reader announcement 300ms out, and clearing + // the window in between leaves it naming links the reader can no longer + // reach, so it has to be dropped along with them. + void test_clearingTheWindowDropsAQueuedHiddenAnnouncement() + { + auto* pConsole = mpHost->mpConsole.data(); + auto& manager = pConsole->getHyperlinkVisibilityManager(); + QVERIFY(pConsole->clear(qsl("main"))); + + fill(pConsole, qsl("seed"), 3); + const int linkId = appendLink(pConsole); + qApp->processEvents(); + + const int registeredOn = lineContaining(pConsole->buffer, mLinkText); + QVERIFY(registeredOn >= 0); + manager.registerHyperlink(linkId, registeredOn, 0, mLinkText.length(), mLinkText, concealLaterStyling()); + + manager.concealLink(linkId); + QVERIFY2(manager.mPendingHiddenCount > 0, "nothing was queued, so clearing it below proves nothing"); + QVERIFY2(manager.mpAnnouncementTimer->isActive(), "nothing was queued, so clearing it below proves nothing"); + + QVERIFY(pConsole->clear(qsl("main"))); + + QCOMPARE(manager.mPendingHiddenCount, 0); + QVERIFY2(!manager.mpAnnouncementTimer->isActive(), "the announcement still fires after the links it counts have gone"); + } + + // The main console's model outlives the view built on it and keeps taking + // lines meanwhile, so the tracked links have to follow a deletion whether or + // not anyone is watching. ~TConsole is what detaches it in production. + void test_deletingALineMovesTrackedLinksWithNoViewAttached() + { + auto* pConsole = mpHost->mpConsole.data(); + auto& buffer = pConsole->buffer; + auto& manager = pConsole->getHyperlinkVisibilityManager(); + QVERIFY(pConsole->clear(qsl("main"))); + + fill(pConsole, qsl("seed"), 3); + const int linkId = appendLink(pConsole); + fill(pConsole, qsl("trailing"), 3); + qApp->processEvents(); + + const int registeredOn = lineContaining(buffer, mLinkText); + QVERIFY2(registeredOn > 0, "the link has to sit below the line deleted here"); + manager.registerHyperlink(linkId, registeredOn, 0, mLinkText.length(), mLinkText, concealLaterStyling()); + + // blanks the link's text, so the reveal below is the only thing that can + // put it back and where it lands is what the stored line number says + manager.concealLink(linkId); + QVERIFY2(lineContaining(buffer, mLinkText) < 0, "the link's text is still in the buffer, so a reveal would prove nothing"); + + buffer.detachConsole(pConsole); + buffer.deleteLine(0); + buffer.setConsole(pConsole); + + manager.revealLink(linkId); + qApp->processEvents(); + QCOMPARE(lineContaining(buffer, mLinkText), registeredOn - 1); + } + private: // A reveal needs a delay or an expire trigger, otherwise registerHyperlink() // refuses to conceal it - nothing would ever reveal it again. Its 100ms poll From 532de8224e09763413346fbb77183012bfcf5732 Mon Sep 17 00:00:00 2001 From: Vadim Peretokin Date: Tue, 1 Sep 2026 11:19:54 +0200 Subject: [PATCH 2/2] fix: only maintain the tracked links of the buffer the manager owns A scratch buffer built with a console back-pointer clears itself in its constructor, which reached through to the live console's manager and dropped every tracked link on it. Assisted-by: Claude:claude-opus-5 --- src/TBuffer.cpp | 17 +++--- test/functional_tests/TrackedLinkTrimTest.cpp | 61 ++++++++++++++++++- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/TBuffer.cpp b/src/TBuffer.cpp index f2618628564..c747756c83d 100644 --- a/src/TBuffer.cpp +++ b/src/TBuffer.cpp @@ -5664,17 +5664,20 @@ bool TBuffer::replaceInLine(QPoint& P_begin, QPoint& P_end, const QString& with, return true; } -// The main console's model outlives the view built on it and keeps taking -// lines meanwhile, so going through mpConsole alone would stop maintaining the -// tracked links too early. Every other model is owned by its view and a -// clipboard slice has no model at all, which is where the nullptr comes from. +// Both branches check that this buffer is the one the manager tracks: a +// scratch buffer can carry a console back-pointer, and every model builds its +// buffer before a view attaches, so reaching a manager on the strength of the +// pointer alone would let one buffer drop another's links. THyperlinkVisibilityManager* TBuffer::hyperlinkVisibilityManagerOrNull() { if (mpConsole) { - return &mpConsole->getHyperlinkVisibilityManager(); + return (this == &mpConsole->buffer) ? &mpConsole->getHyperlinkVisibilityManager() : nullptr; } - // mainConsoleModelOrNull() rather than mainConsoleModel() because this also - // runs from the TBuffer constructor, before Host holds the model. + // The main console's model outlives the view built on it and keeps taking + // lines meanwhile, so stopping at mpConsole would give up maintaining its + // links too early. mainConsoleModelOrNull() rather than mainConsoleModel() + // because this also runs from the TBuffer constructor, before Host holds + // the model. TConsoleModel* pModel = mpHost.isNull() ? nullptr : mpHost->mainConsoleModelOrNull(); return (pModel && this == &pModel->buffer) ? &pModel->mHyperlinkVisibilityManager : nullptr; } diff --git a/test/functional_tests/TrackedLinkTrimTest.cpp b/test/functional_tests/TrackedLinkTrimTest.cpp index 7b334df7253..508a27066db 100644 --- a/test/functional_tests/TrackedLinkTrimTest.cpp +++ b/test/functional_tests/TrackedLinkTrimTest.cpp @@ -120,13 +120,16 @@ private slots: { auto& manager = mpHost->mpConsole->getHyperlinkVisibilityManager(); + const int firstRemoved = 0; const int lastRemoved = csmBatchDeleteSize - 1; const int firstSurvivor = csmBatchDeleteSize; + QVERIFY(manager.registerHyperlink(4, firstRemoved, 0, mLinkText.length(), mLinkText, concealedRevealStyling())); QVERIFY(manager.registerHyperlink(5, lastRemoved, 0, mLinkText.length(), mLinkText, concealedRevealStyling())); QVERIFY(manager.registerHyperlink(6, firstSurvivor, 0, mLinkText.length(), mLinkText, concealedRevealStyling())); manager.adjustLineNumbers(0, csmBatchDeleteSize); + QVERIFY2(!manager.trackedLinkIds().contains(4), "the link on the first removed line was kept, so entries accumulate"); QVERIFY2(!manager.trackedLinkIds().contains(5), "the link on the last removed line was kept, so entries accumulate"); QVERIFY2(manager.trackedLinkIds().contains(6), "the link on the first surviving line was dropped, so it can never reveal"); QCOMPARE(manager.mTrackedLinks[6].lineNumber, 0); @@ -164,9 +167,7 @@ private slots: } // A wholeline concealment deletes the link's own line, so every link below - // it moves up exactly one. Driven through onLinkClicked() because a click is - // the only thing that conceals without a delay, and there is no Lua entry - // point for one - see TBufferOSC_spec.lua, which says as much. + // it moves up exactly one. void test_aWholelineConcealmentMovesTheLinksBelowItOnlyOnce() { auto* pConsole = mpHost->mpConsole.data(); @@ -292,6 +293,60 @@ private slots: QCOMPARE(lineContaining(buffer, mLinkText), registeredOn - 1); } + // A buffer clears itself as it is built, and every buffer of the host that + // is not the one a manager tracks has to leave that manager alone - a copy + // or a cut would otherwise drop every link on the console it was taken from. + void test_anotherBuffersLifecycleLeavesTheTrackedLinksAlone() + { + auto* pConsole = mpHost->mpConsole.data(); + auto& manager = pConsole->getHyperlinkVisibilityManager(); + QVERIFY(pConsole->clear(qsl("main"))); + + fill(pConsole, qsl("seed"), 3); + const int linkId = appendLink(pConsole); + qApp->processEvents(); + + const int registeredOn = lineContaining(pConsole->buffer, mLinkText); + QVERIFY(registeredOn >= 0); + QVERIFY(manager.registerHyperlink(linkId, registeredOn, 0, mLinkText.length(), mLinkText, concealedRevealStyling())); + + // what a copy or a cut builds + TBuffer viewlessSlice(mpHost); + QVERIFY2(manager.trackedLinkIds().contains(linkId), "building a viewless buffer dropped another buffer's links"); + + // a scratch buffer handed a console it does not belong to + TBuffer consoleBoundScratch(mpHost, pConsole); + QVERIFY2(manager.trackedLinkIds().contains(linkId), "building a scratch buffer dropped the console's links"); + + consoleBoundScratch.deleteLine(0); + QCOMPARE(manager.mTrackedLinks[linkId].lineNumber, registeredOn); + } + + // Every console has its own model and its own manager, so a miniconsole has + // to maintain its links off its own buffer rather than the main one's. + void test_aMiniconsoleMaintainsItsOwnTrackedLinks() + { + auto* pMain = mpHost->mpConsole.data(); + auto* pMini = pMain->createMiniConsole(QString(), qsl("trackedLinkMini"), 0, 0, 300, 100); + QVERIFY2(pMini, "the miniconsole was not created, so this test proves nothing"); + QVERIFY2(&pMini->getHyperlinkVisibilityManager() != &pMain->getHyperlinkVisibilityManager(), "the miniconsole shares the main console's manager"); + + auto& buffer = pMini->buffer; + auto& manager = pMini->getHyperlinkVisibilityManager(); + + fill(pMini, qsl("seed"), 3); + const int linkId = appendLink(pMini); + qApp->processEvents(); + + const int registeredOn = lineContaining(buffer, mLinkText); + QVERIFY2(registeredOn > 0, "the link has to sit below the line deleted here"); + QVERIFY(manager.registerHyperlink(linkId, registeredOn, 0, mLinkText.length(), mLinkText, concealedRevealStyling())); + + buffer.deleteLine(0); + + QCOMPARE(manager.mTrackedLinks[linkId].lineNumber, registeredOn - 1); + } + private: // A reveal needs a delay or an expire trigger, otherwise registerHyperlink() // refuses to conceal it - nothing would ever reveal it again. Its 100ms poll