From 8ec72d6906108dd3c6cda1cc669e2ca668c3adac Mon Sep 17 00:00:00 2001
From: mikiubo <michele.uboldi@gmail.com>
Date: Mon, 7 Apr 2025 23:13:31 +0200
Subject: [PATCH] LibUnicode: Avoid rejecting end-of-text position as a valid
 boundary

When the cursor was positioned at the end of text,
attempting to move it left(using the left arrow key)
would fail because align_boundary() was rejecting
the end-of-text position as a valid boundary.
---
 Libraries/LibUnicode/Segmenter.cpp | 28 ++++++++++++----------------
 Tests/LibUnicode/TestSegmenter.cpp | 28 ++++++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/Libraries/LibUnicode/Segmenter.cpp b/Libraries/LibUnicode/Segmenter.cpp
index db66f9e5516..63023995d88 100644
--- a/Libraries/LibUnicode/Segmenter.cpp
+++ b/Libraries/LibUnicode/Segmenter.cpp
@@ -87,15 +87,13 @@ public:
     virtual Optional<size_t> previous_boundary(size_t boundary, Inclusive inclusive) override
     {
         auto icu_boundary = align_boundary(boundary);
-        if (!icu_boundary.has_value())
-            return {};
 
         if (inclusive == Inclusive::Yes) {
-            if (static_cast<bool>(m_segmenter->isBoundary(*icu_boundary)))
-                return static_cast<size_t>(*icu_boundary);
+            if (static_cast<bool>(m_segmenter->isBoundary(icu_boundary)))
+                return static_cast<size_t>(icu_boundary);
         }
 
-        if (auto index = m_segmenter->preceding(*icu_boundary); index != icu::BreakIterator::DONE)
+        if (auto index = m_segmenter->preceding(icu_boundary); index != icu::BreakIterator::DONE)
             return static_cast<size_t>(index);
 
         return {};
@@ -104,15 +102,13 @@ public:
     virtual Optional<size_t> next_boundary(size_t boundary, Inclusive inclusive) override
     {
         auto icu_boundary = align_boundary(boundary);
-        if (!icu_boundary.has_value())
-            return {};
 
         if (inclusive == Inclusive::Yes) {
-            if (static_cast<bool>(m_segmenter->isBoundary(*icu_boundary)))
-                return static_cast<size_t>(*icu_boundary);
+            if (static_cast<bool>(m_segmenter->isBoundary(icu_boundary)))
+                return static_cast<size_t>(icu_boundary);
         }
 
-        if (auto index = m_segmenter->following(*icu_boundary); index != icu::BreakIterator::DONE)
+        if (auto index = m_segmenter->following(icu_boundary); index != icu::BreakIterator::DONE)
             return static_cast<size_t>(index);
 
         return {};
@@ -177,25 +173,25 @@ public:
     }
 
 private:
-    Optional<i32> align_boundary(size_t boundary)
+    i32 align_boundary(size_t boundary)
     {
         auto icu_boundary = static_cast<i32>(boundary);
 
         return m_segmented_text.visit(
-            [&](String const& text) -> Optional<i32> {
+            [&](String const& text) {
                 if (boundary >= text.byte_count())
-                    return {};
+                    return static_cast<i32>(text.byte_count());
 
                 U8_SET_CP_START(text.bytes().data(), 0, icu_boundary);
                 return icu_boundary;
             },
-            [&](icu::UnicodeString const& text) -> Optional<i32> {
+            [&](icu::UnicodeString const& text) {
                 if (icu_boundary >= text.length())
-                    return {};
+                    return text.length();
 
                 return text.getChar32Start(icu_boundary);
             },
-            [](Empty) -> Optional<i32> { VERIFY_NOT_REACHED(); });
+            [](Empty) -> i32 { VERIFY_NOT_REACHED(); });
     }
 
     void for_each_boundary(SegmentationCallback callback)
diff --git a/Tests/LibUnicode/TestSegmenter.cpp b/Tests/LibUnicode/TestSegmenter.cpp
index d40ea82cebf..13368ab8451 100644
--- a/Tests/LibUnicode/TestSegmenter.cpp
+++ b/Tests/LibUnicode/TestSegmenter.cpp
@@ -136,11 +136,23 @@ TEST_CASE(out_of_bounds)
         auto segmenter = Unicode::Segmenter::create(Unicode::SegmenterGranularity::Word);
         segmenter->set_segmented_text(text);
 
-        auto result = segmenter->previous_boundary(text.byte_count());
+        auto result = segmenter->previous_boundary(text.byte_count() + 1);
+        EXPECT(result.has_value());
+
+        result = segmenter->next_boundary(text.byte_count() + 1);
         EXPECT(!result.has_value());
 
+        result = segmenter->previous_boundary(text.byte_count());
+        EXPECT(result.has_value());
+
         result = segmenter->next_boundary(text.byte_count());
         EXPECT(!result.has_value());
+
+        result = segmenter->next_boundary(0);
+        EXPECT(result.has_value());
+
+        result = segmenter->previous_boundary(0);
+        EXPECT(!result.has_value());
     }
     {
         auto text = MUST(AK::utf8_to_utf16("foo"sv));
@@ -148,10 +160,22 @@ TEST_CASE(out_of_bounds)
         auto segmenter = Unicode::Segmenter::create(Unicode::SegmenterGranularity::Word);
         segmenter->set_segmented_text(Utf16View { text });
 
-        auto result = segmenter->previous_boundary(text.size());
+        auto result = segmenter->previous_boundary(text.size() + 1);
+        EXPECT(result.has_value());
+
+        result = segmenter->next_boundary(text.size() + 1);
         EXPECT(!result.has_value());
 
+        result = segmenter->previous_boundary(text.size());
+        EXPECT(result.has_value());
+
         result = segmenter->next_boundary(text.size());
         EXPECT(!result.has_value());
+
+        result = segmenter->next_boundary(0);
+        EXPECT(result.has_value());
+
+        result = segmenter->previous_boundary(0);
+        EXPECT(!result.has_value());
     }
 }