Fix inline code being rendered as blocks. (#5017)

* Fix inline code being rendered as blocks. And make blocks non-greedy as well as only scrolling when needed. * Rename the bubble layout priorities. * Add an InlineCode attribute so that the builder also strips links from these too. * Split up the snapshot tests into individual cases. This should make it much easier to see *what* has changed when regenerating.
2026-11-04 10:33:45 +00:00 · 2026-01-28 11:46:13 +00:00
parent 3d5269fa19
commit fa200cfedb
67 changed files with 324 additions and 83 deletions
@@ -32,6 +32,7 @@ extension NSAttributedString.Key {
    static let MatrixEventOnRoomAlias: NSAttributedString.Key = .init(rawValue: EventOnRoomAliasAttribute.name)
    static let MatrixAllUsersMention: NSAttributedString.Key = .init(rawValue: AllUsersMentionAttribute.name)
    static let CodeBlock: NSAttributedString.Key = .init(rawValue: CodeBlockAttribute.name)
+    static let InlineCode: NSAttributedString.Key = .init(rawValue: InlineCodeAttribute.name)
 }

 struct AttributedStringBuilder: AttributedStringBuilderProtocol {
@@ -197,15 +198,24 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
                content.addAttribute(.MatrixBlockquote, value: true, range: NSRange(location: 0, length: content.length))
                
            case "code", "pre":
-                let preserveFormatting = preserveFormatting || tag == "pre"
+                let isCodeBlock = tag == "pre"
+                
+                let preserveFormatting = preserveFormatting || isCodeBlock
                content = attributedString(element: childElement, documentBody: documentBody, preserveFormatting: preserveFormatting, listTag: listTag, listIndex: &childIndex, indentLevel: indentLevel)
                
                let fontPointSize = fontPointSize * 0.9 // Intentionally shrink code blocks by 10%
                content.setFontPreservingSymbolicTraits(UIFont.monospacedSystemFont(ofSize: fontPointSize, weight: .regular))
                
-                content.addAttribute(.CodeBlock, value: true, range: NSRange(location: 0, length: content.length))
+                if isCodeBlock {
+                    content.addAttribute(.CodeBlock, value: true, range: NSRange(location: 0, length: content.length))
+                    // The scroll view provides the background colour for code blocks.
+                } else {
+                    content.addAttribute(.InlineCode, value: true, range: NSRange(location: 0, length: content.length))
+                    // But inline code is (obviously) inline so it's much easier to set the background colour here.
+                    content.addAttribute(.backgroundColor, value: UIColor.compound._bgCodeBlock as Any, range: NSRange(location: 0, length: content.length))
+                }
                
-                // Don't allow identifiers or links in code blocks
+                // Don't allow identifiers or links in code.
                content.removeAttribute(.MatrixRoomID, range: NSRange(location: 0, length: content.length))
                content.removeAttribute(.MatrixRoomAlias, range: NSRange(location: 0, length: content.length))
                content.removeAttribute(.MatrixUserID, range: NSRange(location: 0, length: content.length))
@@ -353,14 +363,15 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol {
        // Sort the links by length so the longest one always takes priority
        matches.sorted { $0.range.length > $1.range.length }.forEach { [attributedString] match in
            // Don't highlight links within codeblocks
-            let isInCodeBlock = attributedString.attribute(.CodeBlock, at: match.range.location, effectiveRange: nil) != nil
-            if isInCodeBlock {
+            let isCode = attributedString.attribute(.CodeBlock, at: match.range.location, effectiveRange: nil) != nil
+                || attributedString.attribute(.InlineCode, at: match.range.location, effectiveRange: nil) != nil
+            if isCode {
                return
            }
            
            var hasLink = false
            attributedString.enumerateAttribute(.link, in: match.range, options: []) { value, _, stop in
-                if value != nil, !isInCodeBlock {
+                if value != nil, !isCode {
                    hasLink = true
                    stop.pointee = true
                }
@@ -68,6 +68,11 @@ enum CodeBlockAttribute: AttributedStringKey {
    static let name = "MXCodeBlockAttribute"
 }

+enum InlineCodeAttribute: AttributedStringKey {
+    typealias Value = Bool
+    static let name = "MXInlineCodeAttribute"
+}
+
 // periphery: ignore - required to make NSAttributedString to AttributedString conversion even if not used directly
 extension AttributeScopes {
    struct ElementXAttributes: AttributeScope {
@@ -84,6 +89,7 @@ extension AttributeScopes {
        let allUsersMention: AllUsersMentionAttribute
        
        let codeBlock: CodeBlockAttribute
+        let inlineCode: InlineCodeAttribute
        
        let swiftUI: SwiftUIAttributes
        let uiKit: UIKitAttributes
@@ -15,7 +15,8 @@ enum HTMLFixtures: String, CaseIterable {
    case textFormatting
    case groupedBlockQuotes
    case separatedBlockQuotes
-    case codeBlocks
+    case code
+    case wideCodeBlock
    case unorderedList
    case orderedList
    
@@ -74,9 +75,9 @@ enum HTMLFixtures: String, CaseIterable {
            <blockquote>Some other blockquote</blockquote>\
            Text after second blockquote
            """
-        case .codeBlocks:
+        case .code:
            """
-            <pre>A preformatted code block
+            <pre>A pre-formatted code block
            <code>struct ContentView: View {
                var body: some View {
                    VStack {
@@ -88,11 +89,17 @@ enum HTMLFixtures: String, CaseIterable {
                    .padding()
                }
            }</code></pre></br>
-            Followed by some plain code blocks</br>
-            <code>Hello, world!</code>
-            <code><b>Hello</b>, <i>world!</i></code>
-            <code><a href="https://www.matrix.org">This link should not be interpreted as such</a></code>
-            <code>And this https://www.matrix.org should be not highlighted</code>
+            Followed by some inline code</br>
+            <p>Plain text <code>code here</code> more text</p>
+            <p><code>Hello, world!</code></p>
+            <p><code><b>Hello</b>, <i>world!</i></code></p>
+            <p><code>&lt;b&gt;Hello&lt;/b&gt;, &lt;i&gt;world!&lt;/i&gt;</code></p>
+            <p><code><a href="https://www.matrix.org">This link should not be interpreted as such</a></code></p>
+            <p><code>And this https://www.matrix.org should be not highlighted</code></p>
+            """
+        case .wideCodeBlock:
+            """
+            <pre><code>CHHapticPattern.mm:487   +[CHHapticPattern patternForKey:error:]: Failed to read pattern library data: Error Domain=NSCocoaErrorDomain Code=260 "The file “hapticpatternlibrary.plist” couldn’t be opened because there is no such file." UserInfo={NSFilePath=/Library/Audio/Tunings/Generic/Haptics/Library/hapticpatternlibrary.plist, NSURL=file:///Library/Audio/Tunings/Generic/Haptics/Library/hapticpatternlibrary.plist, NSUnderlyingError=0x600000da69d0 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}}</code></pre>
            """
        case .unorderedList:
            """
@@ -10,8 +10,8 @@ import SwiftUI

 /// A custom layout used for quotes and content when using the bubbles timeline style.
 ///
-/// A custom layout is required as the embedded quote bubbles should fill the entire width of
-/// the message bubble, without causing the width of the bubble to fill all of the available space.
+/// A custom layout is required as the embedded quote bubbles and code blocks should fill the entire
+/// width of the message bubble, without causing the width of the bubble to fill all of the available space.
 struct TimelineBubbleLayout: Layout {
    struct Cache {
        var sizes = [Int: [ProposedViewSize: CGSize]]()
@@ -24,12 +24,14 @@ struct TimelineBubbleLayout: Layout {
    /// `TimelineBubbleLayout` to create the layout we would like. They aren't
    /// used in the expected way that SwiftUI would normally use layout priorities.
    enum Priority {
-        /// The priority of hidden quote bubbles that are only used for layout calculations.
-        static let hiddenQuote: Double = -1
-        /// The priority of visible quote bubbles that are placed in the view with a full width.
-        static let visibleQuote: Double = 0
+        /// The priority of hidden quote bubbles/code blocks that are only used for layout calculations.
+        /// Any views given this priority should be made non-greedy for the calculations to work.
+        static let hiddenGreedyComponent: Double = -1
+        /// The priority of visible quote bubbles/code blocks that are placed in the view with a full width.
+        /// Any views given this priority should remain in their normal greedy form.
+        static let visibleGreedyComponent: Double = 0
        /// The priority of regular text that is used for layout calculations and placed in the view.
-        static let regularText: Double = 1
+        static let nonGreedyComponent: Double = 1
    }
    
    func makeCache(subviews: Subviews) -> Cache {
@@ -45,7 +47,7 @@ struct TimelineBubbleLayout: Layout {
        guard !subviews.isEmpty else { return .zero }
        
        // Calculate the natural size using the regular text and non-greedy quote bubbles.
-        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleQuote }
+        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleGreedyComponent }

        let subviewSizes = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache) }
        
@@ -59,12 +61,12 @@ struct TimelineBubbleLayout: Layout {
    func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout Cache) {
        guard !subviews.isEmpty else { return }
        
-        // Calculate the width using the regular text and the non-greedy quote bubbles.
-        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleQuote }
+        // Calculate the width using the regular text along with non-greedy versions of any greedy components.
+        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleGreedyComponent }
        let maxWidth = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache).width }.reduce(0, max)
        
-        // Place the regular text and greedy quote bubbles using the calculated width.
-        let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenQuote }
+        // Place the regular text and greedy components using the calculated width.
+        let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenGreedyComponent }

        let subviewSizes = visibleSubviews.map { size(for: $0, subviews: subviews, proposedSize: ProposedViewSize(width: maxWidth, height: proposal.height), cache: &cache) }
        
@@ -190,7 +190,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
            if !context.viewState.timelineKind.isThread, timelineItem.properties.isThreaded {
                ThreadDecorator()
                    .padding(.leading, 4)
-                    .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                    .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
            }
            
            if let replyDetails = timelineItem.properties.replyDetails {
@@ -203,7 +203,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
                    .frame(maxWidth: .infinity, alignment: .leading)
                    .background(Color.compound.bgCanvasDefault)
                    .cornerRadius(8)
-                    .layoutPriority(TimelineBubbleLayout.Priority.visibleQuote)
+                    .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent)
                    .onTapGesture {
                        if context.viewState.timelineKind != .pinned {
                            context.send(viewAction: .focusOnEventID(replyDetails.eventID))
@@ -214,12 +214,12 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
                TimelineReplyView(placement: .timeline, timelineItemReplyDetails: replyDetails)
                    .fixedSize(horizontal: false, vertical: true)
                    .padding(4.0)
-                    .layoutPriority(TimelineBubbleLayout.Priority.hiddenQuote)
+                    .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent)
                    .hidden()
            }
            
            content()
-                .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
                .cornerRadius(timelineItem.contentCornerRadius)
        }
    }
@@ -97,18 +97,20 @@ struct FormattedBodyText: View {
                                    .foregroundColor(.compound.textSecondary)
                                    .padding(.vertical, 2)
                            }
-                            .layoutPriority(TimelineBubbleLayout.Priority.visibleQuote)
+                            .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent)
                    case .codeBlock:
+                        // The rendered codeblock with a greedy width (due to the scroll view). The custom
+                        // layout prevents the scroll view from increasing the overall width of the view.
                        ScrollView(.horizontal) {
                            MessageText(attributedString: component.attributedString)
                                .padding([.horizontal, .top], 4)
                                .padding(.bottom, 8)
                        }
                        .background(.compound._bgCodeBlock)
+                        .scrollBounceBehavior(.basedOnSize, axes: .horizontal)
                        .scrollIndicatorsFlash(onAppear: true)
-                        .fixedSize(horizontal: false, vertical: true)
                        .padding(.horizontal, 4)
-                        .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                        .layoutPriority(TimelineBubbleLayout.Priority.visibleGreedyComponent)
                        .contextMenu {
                            Button(L10n.actionCopy) {
                                UIPasteboard.general.string = component.attributedString.string
@@ -118,20 +120,27 @@ struct FormattedBodyText: View {
                        MessageText(attributedString: component.attributedString)
                            .padding(.horizontal, 4)
                            .fixedSize(horizontal: false, vertical: true)
-                            .layoutPriority(TimelineBubbleLayout.Priority.regularText)
+                            .layoutPriority(TimelineBubbleLayout.Priority.nonGreedyComponent)
                    }
                }
            }
            
-            // Make a second iteration through the components adding fixed width blockquotes
+            // Make a second iteration through the components adding fixed width blockquotes/codeblocks
            // which are used for layout calculations but won't be rendered.
            ForEach(attributedComponents) { component in
-                if case .blockquote = component.type {
+                switch component.type {
+                case .blockquote:
                    MessageText(attributedString: component.attributedString.mergingAttributes(blockquoteAttributes))
                        .fixedSize(horizontal: false, vertical: true)
                        .padding(.leading, 12.0)
-                        .layoutPriority(TimelineBubbleLayout.Priority.hiddenQuote)
+                        .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent)
                        .hidden()
+                case .codeBlock:
+                    HiddenCodeBlockScrollView(attributedString: component.attributedString)
+                        .layoutPriority(TimelineBubbleLayout.Priority.hiddenGreedyComponent)
+                        .hidden()
+                case .plainText:
+                    EmptyView()
                }
            }
        }
@@ -147,51 +156,101 @@ struct FormattedBodyText: View {
        
        return container
    }
+    
+    /// A self-sizing version of the code block component's view, necessary
+    /// because unlike quote bubbles, code blocks don't wrap when the space
+    /// is constrained.
+    private struct HiddenCodeBlockScrollView: View {
+        let attributedString: AttributedString
+        
+        @State private var maxSize: CGSize = .zero
+        
+        var body: some View {
+            ScrollView(.horizontal) {
+                MessageText(attributedString: attributedString)
+                    .padding([.horizontal, .top], 4)
+                    .padding(.bottom, 8)
+                    .onGeometryChange(for: CGSize.self) { $0.size } action: { maxSize = $0 }
+            }
+            .frame(maxWidth: maxSize.width)
+            .padding(.horizontal, 4)
+        }
+    }
 }

 // MARK: - Previews

 struct FormattedBodyText_Previews: PreviewProvider, TestablePreview {
+    static let attributedStringBuilder = AttributedStringBuilder(cacheKey: "FormattedBodyText", mentionBuilder: MentionBuilder())
    static var previews: some View {
-        body(AttributedStringBuilder(cacheKey: "FormattedBodyText", mentionBuilder: MentionBuilder()))
+        htmlFixtures
+        
+        basicText
            .previewLayout(.sizeThatFits)
+            .previewDisplayName("basicText")
+        
+        singleColumnComponents
+            .previewLayout(.sizeThatFits)
+            .previewDisplayName("singleColumnComponents")
+    }
+    
+    static var basicText: some View {
+        VStack(alignment: .leading, spacing: 4.0) {
+            FormattedBodyText(attributedString: AttributedString("Some plain text wrapped in an AttributedString."))
+                .bubbleBackground()
+            
+            FormattedBodyText(text: "Some plain text that's not an attributed component.")
+                .bubbleBackground()
+            
+            FormattedBodyText(text: "❤️", boostFontSize: true)
+                .bubbleBackground()
+        }
+        .padding()
+    }
+    
+    /// A preview to help ensure that none of the component types we support result
+    /// in a bubble's width becoming wider than the natural width of its contents.
+    @ViewBuilder
+    static var singleColumnComponents: some View {
+        let html = """
+        <blockquote>A</blockquote>
+        <pre><code>B</code></pre>
+        <p>C</p>
+        """
+        
+        if let attributedString = attributedStringBuilder.fromHTML(html) {
+            FormattedBodyText(attributedString: attributedString)
+                .bubbleBackground()
+                .padding(4.0)
+        }
    }
    
    @ViewBuilder
-    static func body(_ attributedStringBuilder: AttributedStringBuilderProtocol) -> some View {
-        let htmlStrings = HTMLFixtures.allCases.map(\.rawValue)
+    static var htmlFixtures: some View {
+        let htmlFixtures = HTMLFixtures.allCases
        
-        ScrollView {
-            VStack(alignment: .leading, spacing: 4.0) {
-                ForEach(htmlStrings, id: \.self) { htmlString in
-                    HStack(alignment: .top, spacing: 0) {
-                        Text(htmlString)
-                            .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
-                            .padding(4.0)
-                        
-                        Divider()
-                            .background(.black)
-                        
-                        if let attributedString = attributedStringBuilder.fromHTML(htmlString) {
-                            FormattedBodyText(attributedString: attributedString)
-                                .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
-                                .bubbleBackground()
-                                .padding(4.0)
-                        }
-                    }
-                    .border(.black)
+        ForEach(htmlFixtures, id: \.rawValue) { htmlFixture in
+            HStack(alignment: .top, spacing: 0) {
+                let htmlString = htmlFixture.rawValue
+                Text(htmlString)
+                    .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
+                    .padding(4.0)
+                
+                Divider()
+                    .background(.black)
+                
+                if let attributedString = attributedStringBuilder.fromHTML(htmlString) {
+                    FormattedBodyText(attributedString: attributedString)
+                        .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading)
+                        .bubbleBackground()
+                        .padding(4.0)
                }
-                
-                FormattedBodyText(attributedString: AttributedString("Some plain text wrapped in an AttributedString."))
-                    .bubbleBackground()
-                
-                FormattedBodyText(text: "Some plain text that's not an attributed component.")
-                    .bubbleBackground()
-                
-                FormattedBodyText(text: "❤️", boostFontSize: true)
-                    .bubbleBackground()
            }
+            .fixedSize(horizontal: false, vertical: true)
+            .border(.black)
            .padding()
+            .previewLayout(.sizeThatFits)
+            .previewDisplayName("\(htmlFixture)")
        }
    }
 }