From ea06b2d46fe76b59b28de6b670906cbe83df7998 Mon Sep 17 00:00:00 2001
From: Donal McBreen <donal@37signals.com>
Date: Fri, 21 Nov 2025 14:25:38 +0000
Subject: [PATCH] Simplify and test highlighing

---
 app/models/search/record/sqlite.rb           | 34 +++++++++++---------
 test/controllers/searches_controller_test.rb | 24 ++++++++++++++
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/app/models/search/record/sqlite.rb b/app/models/search/record/sqlite.rb
index 6c72b3e32..cba6a1cde 100644
--- a/app/models/search/record/sqlite.rb
+++ b/app/models/search/record/sqlite.rb
@@ -6,6 +6,10 @@ module Search::Record::SQLite
     # FTS tables require integer rowids
     attribute :id, :integer, default: nil
 
+    # Virtual attributes from FTS5 functions
+    attribute :result_title, :string
+    attribute :result_content, :string
+
     after_save :upsert_to_fts5_table
     after_destroy :delete_from_fts5_table
 
@@ -21,36 +25,34 @@ module Search::Record::SQLite
       closing_mark = connection.quote(Search::Highlighter::CLOSING_MARK)
       ellipsis = connection.quote(Search::Highlighter::ELIPSIS)
 
-      [ "highlight(search_records_fts, 0, #{opening_mark}, #{closing_mark}) AS highlighted_title",
-        "highlight(search_records_fts, 1, #{opening_mark}, #{closing_mark}) AS highlighted_content",
-        "snippet(search_records_fts, 1, #{opening_mark}, #{closing_mark}, #{ellipsis}, 20) AS content_snippet",
+      [ "highlight(search_records_fts, 0, #{opening_mark}, #{closing_mark}) AS result_title",
+        "snippet(search_records_fts, 1, #{opening_mark}, #{closing_mark}, #{ellipsis}, 20) AS result_content",
         "#{connection.quote(query.terms)} AS query" ]
     end
   end
 
   def card_title
-    if card_id
-      if has_attribute?(:highlighted_title) && highlighted_title.present?
-        highlighted_title.html_safe
-      else
-        card.title
-      end
-    end
+    escape_fts_highlight(result_title || card.title)
   end
 
   def card_description
-    if card_id && has_attribute?(:content_snippet) && content_snippet.present?
-      content_snippet.html_safe
-    end
+    escape_fts_highlight(result_content) unless comment
   end
 
   def comment_body
-    if comment && has_attribute?(:content_snippet) && content_snippet.present?
-      content_snippet.html_safe
-    end
+    escape_fts_highlight(result_content) if comment
   end
 
   private
+    def escape_fts_highlight(html)
+      return nil unless html.present?
+
+      CGI.escapeHTML(html)
+        .gsub(CGI.escapeHTML(Search::Highlighter::OPENING_MARK), Search::Highlighter::OPENING_MARK.html_safe)
+        .gsub(CGI.escapeHTML(Search::Highlighter::CLOSING_MARK), Search::Highlighter::CLOSING_MARK.html_safe)
+        .html_safe
+    end
+
     def upsert_to_fts5_table
       self.class.connection.exec_query(
         "INSERT OR REPLACE INTO search_records_fts(rowid, title, content) VALUES (?, ?, ?)",
diff --git a/test/controllers/searches_controller_test.rb b/test/controllers/searches_controller_test.rb
index 12c191567..98ae5cdd3 100644
--- a/test/controllers/searches_controller_test.rb
+++ b/test/controllers/searches_controller_test.rb
@@ -30,6 +30,7 @@ class SearchesControllerTest < ActionDispatch::IntegrationTest
     assert_select "li .search__title", text: /Just haggis/, count: 2 # card title shows up in two entries
     assert_select "li .search__excerpt", text: /More haggis/ # one entry for the card description
     assert_select "li .search__excerpt--comment", text: /I love haggis/ # one entry for the comment
+    assert_match(/<mark class="circled-text"><span><\/span>haggis<\/mark>/, response.body)
 
     # Searching by card id
     get search_path(q: @card.id, script_name: "/#{@account.external_account_id}")
@@ -40,4 +41,27 @@ class SearchesControllerTest < ActionDispatch::IntegrationTest
     assert_select "form[data-controller='auto-submit']", count: 0
     assert_select ".search__empty", text: "No matches"
   end
+
+  test "search highlights matched terms with proper HTML marks" do
+    @board.cards.create!(title: "Testing search highlighting", creator: @user)
+
+    get search_path(q: "highlighting", script_name: "/#{@account.external_account_id}")
+    assert_response :success
+
+  end
+
+  test "search preserves highlight marks but escapes surrounding HTML" do
+    @board.cards.create!(
+      title: "<b>Bold</b> testing content",
+      creator: @user
+    )
+
+    get search_path(q: "testing", script_name: "/#{@account.external_account_id}")
+    assert_response :success
+
+    # Should escape <b> tags
+    assert response.body.include?("&lt;b&gt;")
+    # But should preserve highlight marks around "testing"
+    assert_match(/<mark class="circled-text"><span><\/span>testing<\/mark>/, response.body)
+  end
 end