diff --git a/src/TBuffer.cpp b/src/TBuffer.cpp index 99d25ac8081..c747756c83d 100644 --- a/src/TBuffer.cpp +++ b/src/TBuffer.cpp @@ -5664,6 +5664,24 @@ bool TBuffer::replaceInLine(QPoint& P_begin, QPoint& P_end, const QString& with, return true; } +// 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 (this == &mpConsole->buffer) ? &mpConsole->getHyperlinkVisibilityManager() : nullptr; + } + // 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; +} + void TBuffer::clear() { // Clearing the display is not gagging: flush any deferred log text before @@ -5699,8 +5717,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 +5946,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..508a27066db 100644 --- a/test/functional_tests/TrackedLinkTrimTest.cpp +++ b/test/functional_tests/TrackedLinkTrimTest.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include "PortableModeTestHelper.h" @@ -119,15 +120,19 @@ 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); } // Concealing a link zeroes its characters' indices, so the scan that decides @@ -161,11 +166,8 @@ 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. void test_aWholelineConcealmentMovesTheLinksBelowItOnlyOnce() { auto* pConsole = mpHost->mpConsole.data(); @@ -231,6 +233,120 @@ 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); + } + + // 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