Repository: RetroAchievements/RAIntegration
Pull request #920 (2022-10-01 18:26:37)

Add warning when serialized size exceeds database limit
![image](https://user-images.githubusercontent.com/32680403/193423130-2d7af05d-400e-48c0-9229-39473be38138.png)


luchaos: Should this mention the `database` constraint, be a local `length` constraint instead?
It'S ultimately a database restriction but I'm not sure how relevant that information is on the client side for creators.
Jamiras: I've removed 'database' from the messages. It now says "Serialized length exceeds limit"

Patch:
diff --git a/src/data/models/AchievementModel.cpp b/src/data/models/AchievementModel.cpp
index c7d29937..417209f5 100644
--- a/src/data/models/AchievementModel.cpp
+++ b/src/data/models/AchievementModel.cpp
@@ -129,6 +129,12 @@ bool AchievementModel::ValidateAsset(std::wstring& sError)
     }
 
     const auto& sTrigger = GetAssetDefinition(m_pTrigger);
+    if (sTrigger.length() > MaxSerializedLength)
+    {
+        sError = ra::StringPrintf(L"Serialized length exceeds limit: %d/%d", sTrigger.length(), MaxSerializedLength);
+        return false;
+    }
+
     return TriggerValidation::Validate(sTrigger, sError, AssetType::Achievement);
 }
 
diff --git a/src/data/models/AchievementModel.hh b/src/data/models/AchievementModel.hh
index fe90954d..6b3934fa 100644
--- a/src/data/models/AchievementModel.hh
+++ b/src/data/models/AchievementModel.hh
@@ -135,6 +135,8 @@ public:
         return s_vValidPoints;
     }
 
+    static constexpr size_t MaxSerializedLength = 65535;
+
 protected:
     void OnValueChanged(const IntModelProperty::ChangeArgs& args) override;
     void OnValueChanged(const StringModelProperty::ChangeArgs& args) override;
diff --git a/src/data/models/LeaderboardModel.cpp b/src/data/models/LeaderboardModel.cpp
index c2cc893d..62b55df2 100644
--- a/src/data/models/LeaderboardModel.cpp
+++ b/src/data/models/LeaderboardModel.cpp
@@ -72,17 +72,11 @@ void LeaderboardModel::OnValueChanged(const IntModelProperty::ChangeArgs& args)
 bool LeaderboardModel::ValidateAsset(std::wstring& sError)
 {
     const auto& sStartTrigger = GetAssetDefinition(m_pStartTrigger);
-
     if (sStartTrigger.empty())
     {
         sError = L"No Start condition";
         return false;
     }
-    if (!TriggerValidation::Validate(sStartTrigger, sError, AssetType::Leaderboard))
-    {
-        sError.insert(0, L"Start: ");
-        return false;
-    }
 
     const auto& sCancelTrigger = GetAssetDefinition(m_pCancelTrigger);
     if (sCancelTrigger.empty())
@@ -90,11 +84,6 @@ bool LeaderboardModel::ValidateAsset(std::wstring& sError)
         sError = L"No Cancel condition";
         return false;
     }
-    if (!TriggerValidation::Validate(sCancelTrigger, sError, AssetType::Leaderboard))
-    {
-        sError.insert(0, L"Cancel: ");
-        return false;
-    }
 
     const auto& sSubmitTrigger = GetAssetDefinition(m_pSubmitTrigger);
     if (sSubmitTrigger.empty())
@@ -102,11 +91,6 @@ bool LeaderboardModel::ValidateAsset(std::wstring& sError)
         sError = L"No Submit condition";
         return false;
     }
-    if (!TriggerValidation::Validate(sSubmitTrigger, sError, AssetType::Leaderboard))
-    {
-        sError.insert(0, L"Submit: ");
-        return false;
-    }
 
     const auto& sValueDefinition = GetAssetDefinition(m_pValueDefinition);
     if (sValueDefinition.empty())
@@ -115,6 +99,32 @@ bool LeaderboardModel::ValidateAsset(std::wstring& sError)
         return false;
     }
 
+    const size_t nSerializedSize = sStartTrigger.length() + sCancelTrigger.length() +
+        sSubmitTrigger.length() + sValueDefinition.length() + 16; // "STA:SUB:CAN:VAL:"
+    if (nSerializedSize > MaxSerializedLength)
+    {
+        sError = ra::StringPrintf(L"Serialized length exceeds limit: %d/%d", nSerializedSize, MaxSerializedLength);
+        return false;
+    }
+
+    if (!TriggerValidation::Validate(sStartTrigger, sError, AssetType::Leaderboard))
+    {
+        sError.insert(0, L"Start: ");
+        return false;
+    }
+
+    if (!TriggerValidation::Validate(sCancelTrigger, sError, AssetType::Leaderboard))
+    {
+        sError.insert(0, L"Cancel: ");
+        return false;
+    }
+
+    if (!TriggerValidation::Validate(sSubmitTrigger, sError, AssetType::Leaderboard))
+    {
+        sError.insert(0, L"Submit: ");
+        return false;
+    }
+
     return true;
 }
 
diff --git a/src/data/models/LeaderboardModel.hh b/src/data/models/LeaderboardModel.hh
index 418d0794..bf9cc1a6 100644
--- a/src/data/models/LeaderboardModel.hh
+++ b/src/data/models/LeaderboardModel.hh
@@ -196,6 +196,8 @@ public:
         m_pCapturedValueDefinitionHits.Reset();
     }
 
+    static constexpr size_t MaxSerializedLength = 65535;
+
 protected:
     void OnValueChanged(const IntModelProperty::ChangeArgs& args) override;
 
diff --git a/src/services/SearchResults.cpp b/src/services/SearchResults.cpp
index 33f7ad63..e84f6840 100644
--- a/src/services/SearchResults.cpp
+++ b/src/services/SearchResults.cpp
@@ -1122,7 +1122,7 @@ public:
 
     std::wstring GetFormattedValue(const SearchResults& pResults, ra::ByteAddress nAddress, MemSize) const override
     {
-        std::array<unsigned char, 16> pBuffer;
+        std::array<unsigned char, 16> pBuffer = {0};
         pResults.GetBytes(nAddress, &pBuffer.at(0), pBuffer.size());
 
         std::wstring sText;
@@ -1524,8 +1524,11 @@ void SearchResults::Initialize(ra::ByteAddress nAddress, size_t nBytes, SearchTy
     m_nType = nType;
 
     const auto& pEmulatorContext = ra::services::ServiceLocator::Get<ra::data::context::EmulatorContext>();
-    if (nBytes + nAddress > pEmulatorContext.TotalMemorySize())
-        nBytes = pEmulatorContext.TotalMemorySize() - nAddress;
+    const auto nTotalMemorySize = pEmulatorContext.TotalMemorySize();
+    if (nAddress > nTotalMemorySize)
+        nAddress = 0;
+    if (nBytes + nAddress > nTotalMemorySize)
+        nBytes = nTotalMemorySize - nAddress;
 
     switch (nType)
     {
@@ -1608,14 +1611,15 @@ void SearchResults::MergeSearchResults(const SearchResults& srMemory, const Sear
         // srAddresses collection.
         auto& pNewBlock = m_vBlocks.emplace_back(pSrcBlock);
         unsigned int nSize = pNewBlock.GetBytesSize();
-        ra::ByteAddress nAddress = pNewBlock.GetFirstAddress();
+        ra::ByteAddress nAddress = m_pImpl->ConvertToRealAddress(pNewBlock.GetFirstAddress());
         unsigned char* pWrite = pNewBlock.GetBytes();
 
         for (const auto& pMemBlock : srMemory.m_vBlocks)
         {
-            if (nAddress >= pMemBlock.GetFirstAddress() && nAddress < pMemBlock.GetFirstAddress() + pMemBlock.GetBytesSize())
+            const auto nBlockFirstAddress = m_pImpl->ConvertToRealAddress(pMemBlock.GetFirstAddress());
+            if (nAddress >= nBlockFirstAddress && nAddress < nBlockFirstAddress + pMemBlock.GetBytesSize())
             {
-                const auto nOffset = nAddress - pMemBlock.GetFirstAddress();
+                const auto nOffset = nAddress - nBlockFirstAddress;
                 const auto nAvailable = pMemBlock.GetBytesSize() - nOffset;
                 if (nAvailable >= nSize)
                 {
diff --git a/src/ui/viewmodels/MemoryBookmarksViewModel.cpp b/src/ui/viewmodels/MemoryBookmarksViewModel.cpp
index b0e14023..8e7dc735 100644
--- a/src/ui/viewmodels/MemoryBookmarksViewModel.cpp
+++ b/src/ui/viewmodels/MemoryBookmarksViewModel.cpp
@@ -11,6 +11,7 @@
 #include "services\IConfiguration.hh"
 #include "services\IFileSystem.hh"
 #include "services\ILocalStorage.hh"
+#include "services\SearchResults.h"
 #include "services\ServiceLocator.hh"
 
 #include "ui\viewmodels\FileDialogViewModel.hh"
@@ -43,6 +44,8 @@ const BoolModelProperty MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadO
 const BoolModelProperty MemoryBookmarksViewModel::MemoryBookmarkViewModel::IsDirtyProperty("MemoryBookmarkViewModel", "IsDirty", false);
 const StringModelProperty MemoryBookmarksViewModel::FreezeButtonTextProperty("MemoryBookmarksViewModel", "FreezeButtonText", L"Freeze");
 
+constexpr int MaxTextBookmarkLength = 8;
+
 MemoryBookmarksViewModel::MemoryBookmarksViewModel() noexcept
 {
     SetWindowTitle(L"Memory Bookmarks");
@@ -59,6 +62,7 @@ MemoryBookmarksViewModel::MemoryBookmarksViewModel() noexcept
     m_vSizes.Add(ra::etoi(MemSize::Nibble_Upper), L"Upper4");
     m_vSizes.Add(ra::etoi(MemSize::Float), L"Float");
     m_vSizes.Add(ra::etoi(MemSize::MBF32), L"MBF32");
+    m_vSizes.Add(ra::etoi(MemSize::Text), L"ASCII");
 
     m_vFormats.Add(ra::etoi(MemFormat::Hex), L"Hex");
     m_vFormats.Add(ra::etoi(MemFormat::Dec), L"Dec");
@@ -93,7 +97,17 @@ void MemoryBookmarksViewModel::MemoryBookmarkViewModel::OnValueChanged(const Int
     else if (args.Property == MemoryBookmarkViewModel::SizeProperty)
     {
         m_nSize = ra::itoe<MemSize>(args.tNewValue);
-        SetReadOnly(m_nSize == MemSize::BitCount);
+        switch (m_nSize)
+        {
+            case MemSize::BitCount:
+            case MemSize::Text:
+                SetReadOnly(true);
+                break;
+
+            default:
+                SetReadOnly(false);
+                break;
+        }
 
         if (m_bInitialized)
         {
@@ -162,6 +176,20 @@ void MemoryBookmarksViewModel::MemoryBookmarkViewModel::OnValueChanged(const Str
 unsigned MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadValue() const
 {
     const auto& pEmulatorContext = ra::services::ServiceLocator::Get<ra::data::context::EmulatorContext>();
+    if (m_nSize == MemSize::Text)
+    {
+        // only have 32 bits to store the value in. generate a hash for the string
+        std::array<uint8_t, MaxTextBookmarkLength + 1> pBuffer;
+        pEmulatorContext.ReadMemory(m_nAddress, &pBuffer.at(0), pBuffer.size() - 1);
+        pBuffer.at(pBuffer.size() - 1) = '\0';
+
+        const char* pText;
+        GSL_SUPPRESS_TYPE1 pText = reinterpret_cast<char*>(&pBuffer.at(0));
+        std::string sText(pText);
+
+        return ra::StringHash(sText);
+    }
+
     return pEmulatorContext.ReadMemory(m_nAddress, m_nSize);
 }
 
@@ -265,6 +293,13 @@ void MemoryBookmarksViewModel::MemoryBookmarkViewModel::UpdateCurrentValue()
 
 std::wstring MemoryBookmarksViewModel::MemoryBookmarkViewModel::BuildCurrentValue() const
 {
+    if (m_nSize == MemSize::Text)
+    {
+        ra::services::SearchResults pResults;
+        pResults.Initialize(m_nAddress, MaxTextBookmarkLength, ra::services::SearchType::AsciiText);
+        return pResults.GetFormattedValue(m_nAddress, MemSize::Text);
+    }
+
     return ra::data::MemSizeFormat(m_nValue, m_nSize, GetFormat());
 }
 
@@ -505,7 +540,8 @@ void MemoryBookmarksViewModel::OnByteWritten(ra::ByteAddress nAddress, uint8_t)
         if (nAddress < nBookmarkAddress)
             continue;
 
-        const auto nBytes = ra::data::MemSizeBytes(pBookmark.GetSize());
+        const auto nSize = pBookmark.GetSize();
+        const auto nBytes = (nSize == MemSize::Text) ? MaxTextBookmarkLength : ra::data::MemSizeBytes(nSize);
         if (nAddress >= nBookmarkAddress + nBytes)
             continue;
 
diff --git a/src/ui/viewmodels/MemorySearchViewModel.cpp b/src/ui/viewmodels/MemorySearchViewModel.cpp
index ec972364..bb7c06f9 100644
--- a/src/ui/viewmodels/MemorySearchViewModel.cpp
+++ b/src/ui/viewmodels/MemorySearchViewModel.cpp
@@ -1070,12 +1070,6 @@ void MemorySearchViewModel::BookmarkSelected()
         return;
 
     const MemSize nSize = m_vSearchResults.back().pResults.GetSize();
-    if (nSize == MemSize::Text)
-    {
-        ra::ui::viewmodels::MessageBoxViewModel::ShowInfoMessage(L"Text bookmarks are not supported");
-        return;
-    }
-
     auto& vmBookmarks = ra::services::ServiceLocator::GetMutable<ra::ui::viewmodels::WindowManager>().MemoryBookmarks;
     if (!vmBookmarks.IsVisible())
         vmBookmarks.Show();
diff --git a/src/ui/win32/MemoryBookmarksDialog.cpp b/src/ui/win32/MemoryBookmarksDialog.cpp
index efd2242f..eb5b6fcb 100644
--- a/src/ui/win32/MemoryBookmarksDialog.cpp
+++ b/src/ui/win32/MemoryBookmarksDialog.cpp
@@ -91,7 +91,7 @@ public:
     HWND CreateInPlaceEditor(HWND hParent, InPlaceEditorInfo& pInfo) override
     {
         const auto& vmItems = static_cast<ra::ui::win32::bindings::GridBinding*>(pInfo.pGridBinding)->GetItems();
-        if (vmItems.GetItemValue(pInfo.nColumnIndex, MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadOnlyProperty))
+        if (vmItems.GetItemValue(pInfo.nItemIndex, MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadOnlyProperty))
             return nullptr;
 
         return ra::ui::win32::bindings::GridTextColumnBinding::CreateInPlaceEditor(hParent, pInfo);
@@ -159,6 +159,7 @@ private:
         {
             case MemSize::Float:
             case MemSize::MBF32:
+            case MemSize::Text:
                 return true;
 
             default:


Test patch:
diff --git a/tests/data/models/AchievementModel_Tests.cpp b/tests/data/models/AchievementModel_Tests.cpp
index 3a8fddce..995428a3 100644
--- a/tests/data/models/AchievementModel_Tests.cpp
+++ b/tests/data/models/AchievementModel_Tests.cpp
@@ -317,6 +317,32 @@ public:
             }
         }
     }
+    
+    TEST_METHOD(TestValidateLength)
+    {
+        AchievementModelHarness achievement;
+        achievement.mockGameContext.SetGameId(22);
+        achievement.AddLocalBadgesModel();
+        achievement.SetBadge(L"12345.png");
+        achievement.CreateServerCheckpoint();
+        achievement.CreateLocalCheckpoint();
+
+        std::string sTrigger;
+        for (int i = 0; i < 65535/12 - 1; i++)
+            sTrigger.append(ra::StringPrintf("0xH00%04X=0_", i));
+        sTrigger.append("0xX00FFF0=12345");
+        Assert::AreEqual({65535U}, sTrigger.length());
+        achievement.SetTrigger(sTrigger);
+
+        Assert::IsTrue(achievement.Validate());
+        Assert::AreEqual(std::wstring(), achievement.GetValidationError());
+
+        sTrigger.append("6"); // 65536 bytes
+        achievement.SetTrigger(sTrigger);
+
+        Assert::IsFalse(achievement.Validate());
+        Assert::AreEqual(std::wstring(L"Serialized length exceeds limit: 65536/65535"), achievement.GetValidationError());
+    }
 };
 
 
diff --git a/tests/services/SearchResults_Tests.cpp b/tests/services/SearchResults_Tests.cpp
index 951303ec..763e7629 100644
--- a/tests/services/SearchResults_Tests.cpp
+++ b/tests/services/SearchResults_Tests.cpp
@@ -1048,6 +1048,40 @@ public:
         Assert::IsTrue(results2.MatchesFilter(results1, result));
     }
 
+    TEST_METHOD(TestInitializeFromResultsThirtyTwoBitAlignedOffset)
+    {
+        std::array<unsigned char, 12> memory{ 0x00, 0x12, 0x34, 0xAB, 0x56, 0xCD, 0x44, 0x20, 0x11, 0x22, 0x33, 0x44 };
+        ra::data::context::mocks::MockEmulatorContext mockEmulatorContext;
+        mockEmulatorContext.MockMemory(memory);
+
+        SearchResults results1;
+        results1.Initialize(4U, 12U, ra::services::SearchType::ThirtyTwoBitAligned);
+        Assert::AreEqual({ 2U }, results1.MatchingAddressCount());
+
+        memory.at(6) = 0x55;
+        SearchResults results2;
+        results2.Initialize(results1, ComparisonType::Equals, ra::services::SearchFilterType::LastKnownValue, L"");
+
+        Assert::AreEqual({ 1U }, results2.MatchingAddressCount());
+        Assert::IsFalse(results2.ContainsAddress(0U));
+        Assert::IsFalse(results2.ContainsAddress(4U));
+        Assert::IsTrue(results2.ContainsAddress(8U));
+
+        SearchResults results;
+        results.Initialize(results1, results2, ComparisonType::Equals, ra::services::SearchFilterType::LastKnownValue, L"");
+
+        Assert::AreEqual({ 1U }, results.MatchingAddressCount());
+        Assert::IsFalse(results.ContainsAddress(0U));
+        Assert::IsFalse(results.ContainsAddress(4U));
+        Assert::IsTrue(results.ContainsAddress(8U));
+
+        SearchResults::Result result;
+        Assert::IsTrue(results.GetMatchingAddress(0U, result));
+        Assert::AreEqual(8U, result.nAddress);
+        Assert::AreEqual(MemSize::ThirtyTwoBit, result.nSize);
+        Assert::AreEqual(0x44332211U, result.nValue);
+    }
+
     TEST_METHOD(TestInitializeFromResultsSixteenBitBigEndianNotEqualPrevious)
     {
         std::array<unsigned char, 5> memory{ 0x00, 0x12, 0x34, 0xAB, 0x56 };
@@ -2046,6 +2080,19 @@ public:
         Assert::IsTrue(results2.MatchesFilter(results1, result2));  // 18 >= 18 (18) ?
         Assert::IsTrue(results2.MatchesFilter(results1, result3));  // 24 >= 18 (24) ?
     }
+
+    TEST_METHOD(TestInitializeFromMemoryAsciiEmpty)
+    {
+        ra::data::context::mocks::MockEmulatorContext mockEmulatorContext;
+        Assert::AreEqual({0U}, mockEmulatorContext.TotalMemorySize());
+
+        SearchResults results;
+        results.Initialize(0U, 16, ra::services::SearchType::AsciiText);
+
+        Assert::AreEqual({0U}, results.MatchingAddressCount());
+
+        Assert::AreEqual(std::wstring(), results.GetFormattedValue(0U, MemSize::Text));
+    }
 };
 
 } // namespace tests
diff --git a/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp b/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp
index 03da5454..d3cabc6b 100644
--- a/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp
@@ -84,7 +84,7 @@ public:
 
         Assert::AreEqual({ 0U }, bookmarks.Bookmarks().Count());
 
-        Assert::AreEqual({ 12U }, bookmarks.Sizes().Count());
+        Assert::AreEqual({ 13U }, bookmarks.Sizes().Count());
         Assert::AreEqual((int)MemSize::EightBit, bookmarks.Sizes().GetItemAt(0)->GetId());
         Assert::AreEqual(std::wstring(L" 8-bit"), bookmarks.Sizes().GetItemAt(0)->GetLabel());
         Assert::AreEqual((int)MemSize::SixteenBit, bookmarks.Sizes().GetItemAt(1)->GetId());
@@ -109,6 +109,8 @@ public:
         Assert::AreEqual(std::wstring(L"Float"), bookmarks.Sizes().GetItemAt(10)->GetLabel());
         Assert::AreEqual((int)MemSize::MBF32, bookmarks.Sizes().GetItemAt(11)->GetId());
         Assert::AreEqual(std::wstring(L"MBF32"), bookmarks.Sizes().GetItemAt(11)->GetLabel());
+        Assert::AreEqual((int)MemSize::Text, bookmarks.Sizes().GetItemAt(12)->GetId());
+        Assert::AreEqual(std::wstring(L"ASCII"), bookmarks.Sizes().GetItemAt(12)->GetLabel());
 
         Assert::AreEqual({ 2U }, bookmarks.Formats().Count());
         Assert::AreEqual((int)MemFormat::Hex, bookmarks.Formats().GetItemAt(0)->GetId());
@@ -1227,10 +1229,17 @@ public:
             const auto nSize = ra::itoe<MemSize>(bookmarks.Sizes().GetItemAt(nIndex)->GetId());
             bookmark.SetSize(nSize);
 
-            if (nSize == MemSize::BitCount)
-                Assert::IsTrue(bookmark.IsReadOnly());
-            else
-                Assert::IsFalse(bookmark.IsReadOnly());
+            switch (nSize)
+            {
+                case MemSize::BitCount:
+                case MemSize::Text:
+                    Assert::IsTrue(bookmark.IsReadOnly());
+                    break;
+
+                default:
+                    Assert::IsFalse(bookmark.IsReadOnly());
+                    break;
+            }
         }
     }
 
@@ -1270,6 +1279,53 @@ public:
         Assert::AreEqual(2U, bookmark.GetChanges());
     }
 
+    TEST_METHOD(TestAddBookmarkASCII)
+    {
+        MemoryBookmarksViewModelHarness bookmarks;
+
+        std::array<uint8_t, 64> memory = {};
+        bookmarks.mockEmulatorContext.MockMemory(memory);
+
+        bookmarks.AddBookmark(1U, MemSize::Text);
+
+        Assert::AreEqual({ 1U }, bookmarks.Bookmarks().Count());
+        const auto& bookmark = *bookmarks.Bookmarks().GetItemAt(0);
+        Assert::AreEqual(std::wstring(L""), bookmark.GetDescription());
+        Assert::AreEqual(1U, bookmark.GetAddress());
+        Assert::AreEqual(MemSize::Text, bookmark.GetSize());
+        Assert::AreEqual(MemoryBookmarksViewModel::BookmarkBehavior::None, bookmark.GetBehavior());
+        Assert::AreEqual(std::wstring(L""), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L""), bookmark.GetPreviousValue());
+        Assert::AreEqual(0U, bookmark.GetChanges());
+
+        memcpy(&memory.at(1), "Test", 5);
+        bookmarks.DoFrame();
+        Assert::AreEqual(std::wstring(L"Test"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L""), bookmark.GetPreviousValue());
+        Assert::AreEqual(1U, bookmark.GetChanges());
+
+        bookmarks.mockEmulatorContext.WriteMemoryByte(3U, 0x78);
+        Assert::AreEqual(std::wstring(L"Text"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"Test"), bookmark.GetPreviousValue());
+        Assert::AreEqual(2U, bookmark.GetChanges());
+
+        memcpy(&memory.at(1), "EightCharacters", 15);
+        bookmarks.DoFrame();
+        Assert::AreEqual(std::wstring(L"EightCha"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"Text"), bookmark.GetPreviousValue());
+        Assert::AreEqual(3U, bookmark.GetChanges());
+
+        bookmarks.mockEmulatorContext.WriteMemoryByte(9U, 0x73);
+        Assert::AreEqual(std::wstring(L"EightCha"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"Text"), bookmark.GetPreviousValue());
+        Assert::AreEqual(3U, bookmark.GetChanges());
+
+        bookmarks.mockEmulatorContext.WriteMemoryByte(8U, 0x73);
+        Assert::AreEqual(std::wstring(L"EightChs"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"EightCha"), bookmark.GetPreviousValue());
+        Assert::AreEqual(4U, bookmark.GetChanges());
+    }
+
     TEST_METHOD(TestSetCurrentValue)
     {
         MemoryBookmarksViewModelHarness bookmarks;
diff --git a/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp b/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp
index e65e8fcb..9f0044b7 100644
--- a/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp
@@ -1276,21 +1276,13 @@ public:
         Assert::AreEqual({ 1U }, search.Results().Count());
         search.Results().GetItemAt(0)->SetSelected(true);
 
-        bool bSawDialog = false;
-        search.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([&bSawDialog](ra::ui::viewmodels::MessageBoxViewModel& vmMessageBox)
-        {
-            bSawDialog = true;
-            Assert::AreEqual(std::wstring(L"Text bookmarks are not supported"), vmMessageBox.GetMessage());
-            return ra::ui::DialogResult::OK;
-        });
-
         search.BookmarkSelected();
-        Assert::IsTrue(bSawDialog);
         Assert::AreEqual({ 1U }, search.Results().Count());
 
         const auto& pBookmarks = search.mockWindowManager.MemoryBookmarks.Bookmarks();
-        Assert::AreEqual({ 0U }, pBookmarks.Count());
-
+        Assert::AreEqual({ 1U }, pBookmarks.Count());
+        Assert::AreEqual({ 8U }, pBookmarks.GetItemAt(0)->GetAddress());
+        Assert::AreEqual(MemSize::Text, pBookmarks.GetItemAt(0)->GetSize());
         Assert::IsTrue(search.Results().GetItemAt(0)->IsSelected());
     }