diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 294813a3f55..cac3718f432 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2279,32 +2279,35 @@ std::wstring TextBuffer::GetHyperlinkUriFromId(uint16_t id) const // - The user-defined id // Return value: // - The internal hyperlink ID -uint16_t TextBuffer::GetHyperlinkId(std::wstring_view params) +uint16_t TextBuffer::GetHyperlinkId(std::wstring_view uri, std::wstring_view id) { - uint16_t id = 0; - if (params.empty()) + uint16_t numericId = 0; + if (id.empty()) { // no custom id specified, return our internal count - id = _currentHyperlinkId; + numericId = _currentHyperlinkId; ++_currentHyperlinkId; } else { // assign _currentHyperlinkId if the custom id does not already exist - const auto result = _hyperlinkCustomIdMap.emplace(params, _currentHyperlinkId); + std::wstring newId{ id }; + // hash the URL and add it to the custom ID - GH#7698 + newId += L"%" + std::to_wstring(std::hash()(uri)); + const auto result = _hyperlinkCustomIdMap.emplace(newId, _currentHyperlinkId); if (result.second) { // the custom id did not already exist ++_currentHyperlinkId; } - id = (*(result.first)).second; + numericId = (*(result.first)).second; } // _currentHyperlinkId could overflow, make sure its not 0 if (_currentHyperlinkId == 0) { ++_currentHyperlinkId; } - return id; + return numericId; } // Method Description: diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 56a3c59f3b0..c72de24517f 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -143,7 +143,7 @@ class TextBuffer final void AddHyperlinkToMap(std::wstring_view uri, uint16_t id); std::wstring GetHyperlinkUriFromId(uint16_t id) const; - uint16_t GetHyperlinkId(std::wstring_view params); + uint16_t GetHyperlinkId(std::wstring_view uri, std::wstring_view id); void RemoveHyperlinkFromMap(uint16_t id); std::wstring GetCustomIdFromId(uint16_t id) const; void CopyHyperlinkMaps(const TextBuffer& OtherBuffer); diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 9a7e896fe95..9fa3a847310 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -574,7 +574,7 @@ CATCH_LOG_RETURN_FALSE() bool Terminal::AddHyperlink(std::wstring_view uri, std::wstring_view params) noexcept { auto attr = _buffer->GetCurrentAttributes(); - const auto id = _buffer->GetHyperlinkId(params); + const auto id = _buffer->GetHyperlinkId(uri, params); attr.SetHyperlinkId(id); _buffer->SetCurrentAttributes(attr); _buffer->AddHyperlinkToMap(uri, id); diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp index fc3db8a8164..08a58ef60d3 100644 --- a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp @@ -37,6 +37,7 @@ namespace TerminalCoreUnitTests TEST_METHOD(AddHyperlink); TEST_METHOD(AddHyperlinkCustomId); + TEST_METHOD(AddHyperlinkCustomIdDifferentUri); }; }; @@ -296,15 +297,45 @@ void TerminalCoreUnitTests::TerminalApiTest::AddHyperlinkCustomId() stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c"); VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url"); - VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); // Send any other text stateMachine.ProcessString(L"Hello World"); VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url"); - VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); // Process the closing osc 8 sequences stateMachine.ProcessString(L"\x1b]8;;\x9c"); VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink()); } + +void TerminalCoreUnitTests::TerminalApiTest::AddHyperlinkCustomIdDifferentUri() +{ + // This is a nearly literal copy-paste of ScreenBufferTests::TestAddHyperlinkCustomId, adapted for the Terminal + + Terminal term; + DummyRenderTarget emptyRT; + term.Create({ 100, 100 }, 0, emptyRT); + + auto& tbi = *(term._buffer); + auto& stateMachine = *(term._stateMachine); + + // Process the opening osc 8 sequence + stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c"); + VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url"); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + + const auto oldAttributes{ tbi.GetCurrentAttributes() }; + + // Send any other text + stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x9c"); + VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"other.url"); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"other.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + + // This second URL should not change the URL of the original ID! + VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(oldAttributes.GetHyperlinkId()), L"test.url"); + VERIFY_ARE_NOT_EQUAL(oldAttributes.GetHyperlinkId(), tbi.GetCurrentAttributes().GetHyperlinkId()); +} diff --git a/src/host/getset.cpp b/src/host/getset.cpp index b7c8e254a0e..7a088afb638 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1567,7 +1567,7 @@ void DoSrvAddHyperlink(SCREEN_INFORMATION& screenInfo, const std::wstring_view params) { auto attr = screenInfo.GetAttributes(); - const auto id = screenInfo.GetTextBuffer().GetHyperlinkId(params); + const auto id = screenInfo.GetTextBuffer().GetHyperlinkId(uri, params); attr.SetHyperlinkId(id); screenInfo.GetTextBuffer().SetCurrentAttributes(attr); screenInfo.GetTextBuffer().AddHyperlinkToMap(uri, id); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 1e559505914..d324def26d1 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -213,6 +213,7 @@ class ScreenBufferTests TEST_METHOD(TestAddHyperlink); TEST_METHOD(TestAddHyperlinkCustomId); + TEST_METHOD(TestAddHyperlinkCustomIdDifferentUri); TEST_METHOD(UpdateVirtualBottomWhenCursorMovesBelowIt); @@ -5956,19 +5957,46 @@ void ScreenBufferTests::TestAddHyperlinkCustomId() stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c"); VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url"); - VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); // Send any other text stateMachine.ProcessString(L"Hello World"); VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url"); - VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); // Process the closing osc 8 sequences stateMachine.ProcessString(L"\x1b]8;;\x9c"); VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink()); } +void ScreenBufferTests::TestAddHyperlinkCustomIdDifferentUri() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + + // Process the opening osc 8 sequence with a custom id + stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c"); + VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url"); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + + const auto oldAttributes{ tbi.GetCurrentAttributes() }; + + // Send any other text + stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x9c"); + VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink()); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"other.url"); + VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"other.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId()); + + // This second URL should not change the URL of the original ID! + VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(oldAttributes.GetHyperlinkId()), L"test.url"); + VERIFY_ARE_NOT_EQUAL(oldAttributes.GetHyperlinkId(), tbi.GetCurrentAttributes().GetHyperlinkId()); +} + void ScreenBufferTests::UpdateVirtualBottomWhenCursorMovesBelowIt() { auto& g = ServiceLocator::LocateGlobals(); diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 1ee79447b08..90c9576197a 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -2464,7 +2464,7 @@ void TextBufferTests::HyperlinkTrim() // Set a hyperlink id in the first row and add a hyperlink to our map const COORD pos{ 70, 0 }; - const auto id = _buffer->GetHyperlinkId(customId); + const auto id = _buffer->GetHyperlinkId(url, customId); TextAttribute newAttr{ 0x7f }; newAttr.SetHyperlinkId(id); _buffer->GetRowByOffset(pos.Y).GetAttrRow().SetAttrToEnd(pos.X, newAttr); @@ -2472,7 +2472,7 @@ void TextBufferTests::HyperlinkTrim() // Set a different hyperlink id somewhere else in the buffer const COORD otherPos{ 70, 5 }; - const auto otherId = _buffer->GetHyperlinkId(otherCustomId); + const auto otherId = _buffer->GetHyperlinkId(otherUrl, otherCustomId); newAttr.SetHyperlinkId(otherId); _buffer->GetRowByOffset(otherPos.Y).GetAttrRow().SetAttrToEnd(otherPos.X, newAttr); _buffer->AddHyperlinkToMap(otherUrl, otherId); @@ -2505,7 +2505,7 @@ void TextBufferTests::NoHyperlinkTrim() // Set a hyperlink id in the first row and add a hyperlink to our map const COORD pos{ 70, 0 }; - const auto id = _buffer->GetHyperlinkId(customId); + const auto id = _buffer->GetHyperlinkId(url, customId); TextAttribute newAttr{ 0x7f }; newAttr.SetHyperlinkId(id); _buffer->GetRowByOffset(pos.Y).GetAttrRow().SetAttrToEnd(pos.X, newAttr);