Skip to content

Commit ffb1ee8

Browse files
committed
Fix HTML parser dropping display names with combining marks
The HTML tokenizer and whitespace collapser operated on Swift Character values (extended grapheme clusters). Unicode combining marks like U+0E4B (Thai Mai Chattawa) following a space or angle bracket merged into a single grapheme cluster, causing: - The tokenizer to fail to find '>' when a combining mark followed it, silently discarding the entire tag and its content. - The whitespace collapser to treat space + combining mark as whitespace, stripping the mark entirely. Both are fixed by operating on unicode scalars instead of grapheme clusters: the tokenizer now scans html.unicodeScalars, and collapseWhitespace() iterates text.unicodeScalars with scalar.properties.isWhitespace. Fixes #146 Assisted-By: OpenCode (Claude)
1 parent 007fba4 commit ffb1ee8

2 files changed

Lines changed: 79 additions & 20 deletions

File tree

Relay/Utilities/MatrixHTMLParser.swift

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,8 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length
273273
// Collapse whitespace in non-preformatted context.
274274
let collapsed = collapseWhitespace(text)
275275
// Suppress whitespace-only text between block elements.
276-
if collapsed.allSatisfy(\.isWhitespace) && result.length > 0 {
276+
if collapsed.unicodeScalars.allSatisfy(\.properties.isWhitespace)
277+
&& result.length > 0 {
277278
let lastChar = result.attributedSubstring(
278279
from: NSRange(location: result.length - 1, length: 1)
279280
).string
@@ -749,28 +750,39 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length
749750

750751
/// Tokenizes HTML into a sequence of text, open-tag, and close-tag tokens.
751752
/// Handles entity decoding, attribute parsing, and self-closing tags.
753+
///
754+
/// Scans using the string's `unicodeScalars` view so that combining marks
755+
/// (e.g. U+0E4B) following `<` or `>` are not merged into a single
756+
/// grapheme cluster with the delimiter, which would cause `Character`-level
757+
/// comparisons like `firstIndex(of: ">")` to fail.
752758
private func tokenize(_ html: String) -> [Token] {
753759
var tokens: [Token] = []
754-
var index = html.startIndex
760+
let scalars = html.unicodeScalars
761+
var index = scalars.startIndex
755762

756-
while index < html.endIndex {
757-
if html[index] == "<" {
763+
while index < scalars.endIndex {
764+
if scalars[index] == "<" {
758765
// Try to parse a tag.
759-
if let tagEnd = html[index...].firstIndex(of: ">") {
760-
let tagContent = html[html.index(after: index)..<tagEnd]
761-
let tagString = String(tagContent).trimmingCharacters(in: .whitespaces)
766+
let afterLT = scalars.index(after: index)
767+
if let tagEnd = scalars[afterLT...].firstIndex(of: ">") {
768+
let tagContent = String(scalars[afterLT..<tagEnd])
769+
let tagString = tagContent.trimmingCharacters(in: .whitespaces)
762770

763771
if tagString.hasPrefix("!--") {
764772
// HTML comment — skip entirely.
765-
if let commentEnd = html[index...].range(of: "-->") {
766-
index = commentEnd.upperBound
773+
if let commentEnd = String(scalars[index...]).range(of: "-->") {
774+
let offset = String(scalars[index...]).distance(
775+
from: String(scalars[index...]).startIndex,
776+
to: commentEnd.upperBound
777+
)
778+
index = scalars.index(index, offsetBy: offset)
767779
} else {
768-
index = html.endIndex
780+
index = scalars.endIndex
769781
}
770782
continue
771783
} else if tagString.hasPrefix("!") || tagString.hasPrefix("?") {
772784
// Doctype or processing instruction — skip.
773-
index = html.index(after: tagEnd)
785+
index = scalars.index(after: tagEnd)
774786
continue
775787
} else if tagString.hasPrefix("/") {
776788
// Close tag.
@@ -792,19 +804,19 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length
792804
// Void tags don't get a close token — handled by not pushing style.
793805
}
794806
}
795-
index = html.index(after: tagEnd)
807+
index = scalars.index(after: tagEnd)
796808
} else {
797809
// Malformed: < without matching >. Treat as text.
798-
tokens.append(.text(String(html[index])))
799-
index = html.index(after: index)
810+
tokens.append(.text(String(scalars[index])))
811+
index = scalars.index(after: index)
800812
}
801813
} else {
802814
// Accumulate text until the next tag.
803815
var textEnd = index
804-
while textEnd < html.endIndex && html[textEnd] != "<" {
805-
textEnd = html.index(after: textEnd)
816+
while textEnd < scalars.endIndex && scalars[textEnd] != "<" {
817+
textEnd = scalars.index(after: textEnd)
806818
}
807-
let rawText = String(html[index..<textEnd])
819+
let rawText = String(scalars[index..<textEnd])
808820
let decoded = decodeHTMLEntities(rawText)
809821
tokens.append(.text(decoded))
810822
index = textEnd
@@ -981,17 +993,23 @@ private struct MatrixHTMLParser { // swiftlint:disable:this type_body_length
981993
// MARK: - Whitespace Helpers
982994

983995
/// Collapses runs of whitespace into single spaces (HTML whitespace normalization).
996+
///
997+
/// Operates on unicode scalars rather than grapheme clusters so that
998+
/// combining marks (e.g. U+0E4B Thai Mai Chattawa) following a space
999+
/// are preserved. Swift groups a space + combining mark into a single
1000+
/// `Character` whose `.isWhitespace` returns `true`, which would
1001+
/// silently discard the combining mark.
9841002
private func collapseWhitespace(_ text: String) -> String {
9851003
var result = ""
9861004
var lastWasSpace = false
987-
for char in text {
988-
if char.isWhitespace || char.isNewline {
1005+
for scalar in text.unicodeScalars {
1006+
if scalar.properties.isWhitespace || scalar == "\n" {
9891007
if !lastWasSpace {
9901008
result.append(" ")
9911009
lastWasSpace = true
9921010
}
9931011
} else {
994-
result.append(char)
1012+
result.unicodeScalars.append(scalar)
9951013
lastWasSpace = false
9961014
}
9971015
}

RelayTests/MatrixHTMLParserTests.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,47 @@ struct MatrixHTMLParserTests {
646646
#expect(firstCharAttrs[.link] == nil)
647647
}
648648

649+
// MARK: - Combining Marks in Whitespace (Issue #146)
650+
651+
@Test func combiningMarkAfterSpacePreserved() {
652+
// U+0E4B (Thai Mai Chattawa) is a combining mark. When it follows
653+
// a space, Swift groups them into a single Character whose
654+
// .isWhitespace returns true. The parser must not discard it.
655+
let html = "a \u{0E4B}b"
656+
let result = NSAttributedString(matrixHTML: html)!
657+
#expect(result.string.contains("\u{0E4B}"))
658+
}
659+
660+
@Test func mentionLinkWithCombiningMarksInDisplayName() {
661+
// Simulates a Matrix mention whose display name contains combining
662+
// marks adjacent to spaces (e.g. "๋࣭ ⭑ username ๋࣭ ⭑").
663+
let displayName = "\u{0E4B}\u{08AD} \u{2B51} username \u{0E4B}\u{08AD} \u{2B51}"
664+
let html = """
665+
<a href="https://matrix.to/#/@user:example.com">\(displayName)</a>
666+
"""
667+
let result = NSAttributedString(matrixHTML: html)
668+
#expect(result != nil, "Parser must not return nil for combining-mark display names")
669+
#expect(result?.string.contains("username") == true)
670+
// The link attribute should be set on the mention text.
671+
if let result {
672+
let link = attrs(result, at: 0)[.link] as? URL
673+
#expect(link?.absoluteString == "https://matrix.to/#/@user:example.com")
674+
}
675+
}
676+
677+
@Test func mentionLinkWithCombiningMarksInContext() {
678+
// A mention with combining marks should not swallow surrounding text.
679+
let html = """
680+
Hello <a href="https://matrix.to/#/@u:e.com">\u{0E4B}\u{08AD} \u{2B51} name</a>!
681+
"""
682+
let result = NSAttributedString(matrixHTML: html)!
683+
#expect(result.string.contains("Hello"))
684+
#expect(result.string.contains("name"))
685+
#expect(result.string.contains("!"))
686+
// The combining mark U+0E4B must not be discarded.
687+
#expect(result.string.unicodeScalars.contains(Unicode.Scalar(0x0E4B)!))
688+
}
689+
649690
// MARK: - Tables
650691

651692
@Test func simpleTable() {

0 commit comments

Comments
 (0)