From 62e1ec046497d08b2dd97889347666abea1fcde7 Mon Sep 17 00:00:00 2001
From: Stefan Ceriu <stefanc@matrix.org>
Date: Fri, 10 May 2024 10:36:20 +0300
Subject: [PATCH] Prevent links from showing up in color within code blocks,
 simplify foreground color stripping

---
 .../HTMLParsing/AttributedStringBuilder.swift | 21 ++++---------------
 .../AttributedStringBuilderTests.swift        | 16 ++++++++------
 2 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift b/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift
index 78ee62f3a..49b03d5a7 100644
--- a/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift
+++ b/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift
@@ -49,9 +49,9 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
         }
 
         let mutableAttributedString = NSMutableAttributedString(string: string)
+        removeDefaultForegroundColors(mutableAttributedString)
         addLinksAndMentions(mutableAttributedString)
         detectPermalinks(mutableAttributedString)
-        removeLinkColors(mutableAttributedString)
         
         let result = try? AttributedString(mutableAttributedString, including: \.elementX)
         Self.cacheValue(result, forKey: string, cacheKey: cacheKey)
@@ -105,12 +105,11 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
         }
         
         let mutableAttributedString = NSMutableAttributedString(attributedString: attributedString)
-        removeDefaultForegroundColor(mutableAttributedString)
+        removeDefaultForegroundColors(mutableAttributedString)
         addLinksAndMentions(mutableAttributedString)
         replaceMarkedBlockquotes(mutableAttributedString)
         replaceMarkedCodeBlocks(mutableAttributedString)
         detectPermalinks(mutableAttributedString)
-        removeLinkColors(mutableAttributedString)
         removeDTCoreTextArtifacts(mutableAttributedString)
         
         let result = try? AttributedString(mutableAttributedString, including: \.elementX)
@@ -140,12 +139,8 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
         return result
     }
     
-    private func removeDefaultForegroundColor(_ attributedString: NSMutableAttributedString) {
-        attributedString.enumerateAttribute(.foregroundColor, in: .init(location: 0, length: attributedString.length), options: []) { value, range, _ in
-            if value as? UIColor == UIColor.black {
-                attributedString.removeAttribute(.foregroundColor, range: range)
-            }
-        }
+    private func removeDefaultForegroundColors(_ attributedString: NSMutableAttributedString) {
+        attributedString.removeAttribute(.foregroundColor, range: .init(location: 0, length: attributedString.length))
     }
     
     private func addLinksAndMentions(_ attributedString: NSMutableAttributedString) {
@@ -293,14 +288,6 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
         }
     }
     
-    private func removeLinkColors(_ attributedString: NSMutableAttributedString) {
-        attributedString.enumerateAttribute(.link, in: .init(location: 0, length: attributedString.length), options: []) { value, range, _ in
-            if value != nil {
-                attributedString.removeAttribute(.foregroundColor, range: range)
-            }
-        }
-    }
-    
     private func removeDTCoreTextArtifacts(_ attributedString: NSMutableAttributedString) {
         guard attributedString.length > 0 else {
             return
diff --git a/UnitTests/Sources/AttributedStringBuilderTests.swift b/UnitTests/Sources/AttributedStringBuilderTests.swift
index da1619921..cf5916e69 100644
--- a/UnitTests/Sources/AttributedStringBuilderTests.swift
+++ b/UnitTests/Sources/AttributedStringBuilderTests.swift
@@ -206,16 +206,18 @@ class AttributedStringBuilderTests: XCTestCase {
         checkLinkIn(attributedString: attributedStringBuilder.fromHTML(string), expectedLink: expectedLink.absoluteString, expectedRuns: 3)
         checkLinkIn(attributedString: attributedStringBuilder.fromPlain(string), expectedLink: expectedLink.absoluteString, expectedRuns: 3)
     }
+    
+    // `Plain link in codeblock: https://www.matrix.org`, Link tag in codeblock: <a href=\"https://www.matrix.org/\">link</a>, plain link: https://www.matrix.org, link tag: <a href=\"https://www.matrix.org/\">link</a>
         
     func testDefaultFont() {
-        let htmlString = "<b>Test</b> <i>string</i>."
+        let htmlString = "<b>Test</b> <i>string</i> "
         
         guard let attributedString = attributedStringBuilder.fromHTML(htmlString) else {
             XCTFail("Could not build the attributed string")
             return
         }
         
-        XCTAssertEqual(attributedString.runs.count, 4)
+        XCTAssertEqual(attributedString.runs.count, 3)
         
         for run in attributedString.runs {
             XCTAssertEqual(run.uiKit.font?.familyName, UIFont.preferredFont(forTextStyle: .body).familyName)
@@ -223,14 +225,14 @@ class AttributedStringBuilderTests: XCTestCase {
     }
     
     func testDefaultForegroundColor() {
-        let htmlString = "<b>Test</b> <i>string</i>."
+        let htmlString = "<b>Test</b> <i>string</i> <a href=\"https://www.matrix.org/\">link</a> <code><a href=\"https://www.matrix.org/\">link</a></code>"
         
         guard let attributedString = attributedStringBuilder.fromHTML(htmlString) else {
             XCTFail("Could not build the attributed string")
             return
         }
         
-        XCTAssertEqual(attributedString.runs.count, 4)
+        XCTAssertEqual(attributedString.runs.count, 7)
         
         for run in attributedString.runs {
             XCTAssertNil(run.uiKit.foregroundColor)
@@ -246,16 +248,18 @@ class AttributedStringBuilderTests: XCTestCase {
             return
         }
         
-        XCTAssertEqual(attributedString.runs.count, 8)
+        XCTAssertEqual(attributedString.runs.count, 3)
         
         var foundLink = false
+        // Foreground colors should be completely stripped from the attributed string
+        // letting UI components chose the defaults (e.g. tintColor)
         for run in attributedString.runs {
             if run.link != nil {
                 XCTAssertEqual(run.link?.host, "www.matrix.org")
                 XCTAssertNil(run.uiKit.foregroundColor)
                 foundLink = true
             } else {
-                XCTAssertNotNil(run.uiKit.foregroundColor)
+                XCTAssertNil(run.uiKit.foregroundColor)
             }
         }