From 2286ca534a037fd46976aa129ad33cc6e0a5397d Mon Sep 17 00:00:00 2001
From: "Jiwei,Yuan" <ji-weiyuan@outlook.com>
Date: Wed, 12 Aug 2026 23:36:18 +0100
Subject: [PATCH] fix(issues): render issue attachments in proportion and play
 videos

GitHub writes the upload's pixel size onto the image tag it pastes into a body,
so a width clamped to the pane against a height still set to the original
stretched every screenshot vertically. Images and video now carry `height: auto`
in both the issue detail and the Markdown reader.

Video did not render at all. `<video>` was not on the sanitizer whitelist, so
raw player markup escaped to visible text; a video upload arrives as a bare
attachment link that only GitHub's own renderer turns into a player, so the pane
makes the same substitution; and playback through the token-authenticating
scheme handler needed real HTTP framing, since WebKit reads media in byte ranges
and a plain URLResponse carries neither status nor `Content-Range`. The handler
answers 206 with the requested slice and holds the asset being seeked, so a
video costs one trip to GitHub instead of one per range.
---
 .../Editor/MarkdownReaderRenderer.swift       |   5 +-
 Sources/termio/Info/MarkdownHTML.swift        |   6 +-
 Sources/termio/Issues/IssuesView.swift        | 131 ++++++++++++++++--
 Tests/termioTests/IssueAttachmentTests.swift  |  56 ++++++++
 Tests/termioTests/MarkdownHTMLTests.swift     |   7 +
 5 files changed, 189 insertions(+), 16 deletions(-)
 create mode 100644 Tests/termioTests/IssueAttachmentTests.swift

diff --git a/Sources/termio/Editor/MarkdownReaderRenderer.swift b/Sources/termio/Editor/MarkdownReaderRenderer.swift
index 3bddfac8..bdaead04 100644
--- a/Sources/termio/Editor/MarkdownReaderRenderer.swift
+++ b/Sources/termio/Editor/MarkdownReaderRenderer.swift
@@ -293,7 +293,10 @@ enum MarkdownReaderRenderer {
     .reader pre code { background: none; padding: 0; font-size: 13.5px; line-height: 1.6; }
     /* The hljs theme ships its own background, padding and base color for `.hljs`; the
        block's look belongs to this stylesheet, so only the token colors survive. */
-    .reader img { max-width: 100%; margin: 0.6em 0; border-radius: 6px; }
+    /* `height: auto` against the pixel height GitHub writes onto a pasted `<img>`; a
+       clamped width with that height still set stretches the picture vertically. */
+    .reader img, .reader video { max-width: 100%; height: auto; margin: 0.6em 0;
+      border-radius: 6px; }
     /* `<kbd>` is on the raw-HTML whitelist and READMEs use it for shortcuts; without a
        key cap it reads as ordinary text. */
     .reader kbd { font: 0.78em var(--font-mono); background: var(--soft);
diff --git a/Sources/termio/Info/MarkdownHTML.swift b/Sources/termio/Info/MarkdownHTML.swift
index 5f3e402b..fa30d1a0 100644
--- a/Sources/termio/Info/MarkdownHTML.swift
+++ b/Sources/termio/Info/MarkdownHTML.swift
@@ -800,16 +800,18 @@ enum HTMLSanitizer {
         "thead", "tbody", "tfoot", "tr", "td", "th", "caption", "blockquote", "dl", "dt",
         "dd", "kbd", "q", "samp", "var", "hr", "s", "summary", "details", "figure",
         "figcaption", "abbr", "cite", "dfn", "mark", "small", "span", "time", "wbr",
-        "picture", "source",
+        "picture", "source", "video",
     ]
 
     /// The useful subset of GitHub's `:all` attribute list plus its per-element ones.
     /// `style`, `class`, `id`, and event handlers are intentionally absent — GitHub
-    /// strips those too.
+    /// strips those too. `autoplay` is absent by choice: a conversation that starts
+    /// playing the moment it opens is hostile in a pane the user only glanced at.
     private static let allowedAttributes: Set<String> = [
         "href", "src", "srcset", "media", "alt", "title", "align", "valign", "width",
         "height", "border", "colspan", "rowspan", "open", "dir", "lang", "start", "type",
         "checked", "disabled", "datetime", "cite", "cellpadding", "cellspacing",
+        "controls", "poster", "muted", "loop", "playsinline", "preload",
     ]
 
     /// `http`/`https`/`mailto`/relative, GitHub's protocol whitelist for href/src.
diff --git a/Sources/termio/Issues/IssuesView.swift b/Sources/termio/Issues/IssuesView.swift
index 28dee0b3..661fa631 100644
--- a/Sources/termio/Issues/IssuesView.swift
+++ b/Sources/termio/Issues/IssuesView.swift
@@ -823,7 +823,7 @@ private struct CapsuleSwitch<Value: Hashable>: View {
 /// `documentMode`, so raw HTML (bot comments, `<picture>`/`<img>`/tables) renders
 /// through the GitHub-mirroring `HTMLSanitizer` whitelist the way GitHub itself
 /// does, colored from the live `TraceTheme`.
-private enum IssueDetailHTML {
+enum IssueDetailHTML {
     static func page(_ detail: IssueDetail, theme: TraceTheme) -> String {
         let s = detail.summary
         let labels = s.labels.map {
@@ -850,12 +850,49 @@ private enum IssueDetailHTML {
         </header>
         <article class="body">\(body)</article>
         \(comments)
+        <script>\(attachmentFallbackScript)</script>
         </body></html>
         """
-        return routeImages(page)
+        return routeImages(embedAttachments(page))
     }
 
-    /// Point every `<img>`/`<source>` at the token-authenticating loader so a private
+    /// GitHub uploads a video as a bare attachment URL on its own line and turns it into a
+    /// player when it renders; the markdown only ever carries the link, so the pane makes the
+    /// same substitution. Images never take this shape — the composer writes them as `<img>`
+    /// or `![…]()` — so a paragraph that is nothing but an attachment link is a video.
+    static func embedAttachments(_ html: String) -> String {
+        let link = #/<p><a href="([^"]+)">[^<]*</a></p>/#
+        return html.replacing(link) { match in
+            let url = String(match.1)
+            guard isAttachment(url) else { return String(match.0) }
+            return "<video class=\"attachment\" controls preload=\"metadata\" src=\"\(url)\"></video>"
+        }
+    }
+
+    /// An uploaded attachment: today's `github.com/user-attachments/assets/<uuid>` (typeless
+    /// in the URL) and the older `*.githubusercontent.com` files, which name their format.
+    private static func isAttachment(_ url: String) -> Bool {
+        guard let parsed = URL(string: url), let host = parsed.host else { return false }
+        let path = parsed.path.lowercased()
+        if host == "github.com" { return path.hasPrefix("/user-attachments/assets/") }
+        guard host.hasSuffix("githubusercontent.com") else { return false }
+        return [".mp4", ".mov", ".webm", ".m4v"].contains { path.hasSuffix($0) }
+    }
+
+    /// The attachment URL carries no type, so a bare link that turns out to be an image
+    /// would render as a dead player. Media that fails to load is one, so swap in the
+    /// image rather than leaving the black box.
+    private static let attachmentFallbackScript = """
+    for (const video of document.querySelectorAll("video.attachment")) {
+      video.addEventListener("error", () => {
+        const image = document.createElement("img");
+        image.src = video.src;
+        video.replaceWith(image);
+      });
+    }
+    """
+
+    /// Point every `<img>`/`<video>`/`<source>` at the token-authenticating loader so a private
     /// repo's attachments resolve — the raw `<img src>` GitHub embeds targets
     /// `github.com/user-attachments/…`, which 404s anonymously and only returns bytes
     /// with the connect token attached (see `GitHubAssetSchemeHandler`). Only `src`/
@@ -912,7 +949,11 @@ private enum IssueDetailHTML {
         pre code { background: none; padding: 0; }
         blockquote { border-left: 3px solid \(theme.secondary); margin-left: 0;
                      padding-left: 10px; color: \(theme.secondary); }
-        img { max-width: 100%; }
+        /* `height: auto` is what keeps a screenshot in proportion: GitHub writes the
+           upload's pixel size onto the image tag, so a width clamped to the pane
+           against a height still set to 1080 stretches the picture vertically. */
+        img, video { max-width: 100%; height: auto; }
+        video.attachment { display: block; border-radius: 6px; }
         table { border-collapse: collapse; }
         td, th { border: 1px solid rgba(128,128,128,.3); padding: 3px 8px; }
         h1, h2, h3, h4 { font-size: 13.5px; margin: 12px 0 6px; }
@@ -974,6 +1015,7 @@ private struct IssueWebView: NSViewRepresentable {
         (view as? IssueDetailWKWebView)?.canAddToChat = canAddToChat
         if context.coordinator.lastHTML != html {
             context.coordinator.lastHTML = html
+            context.coordinator.assetHandler.forgetMedia()
             view.loadHTMLString(html, baseURL: nil)
         }
     }
@@ -1060,6 +1102,12 @@ final class GitHubAssetSchemeHandler: NSObject, WKURLSchemeHandler {
     static let scheme = "x-termio-ghasset"
 
     private var live = Set<ObjectIdentifier>()
+    /// The one asset a media element is currently reading, held whole. WebKit walks a video
+    /// in dozens of tiny byte ranges — the container's header, then its index at the far end,
+    /// then playback — and answering each with its own trip to GitHub rate-limits the pane
+    /// before the first frame appears. Only a ranged request fills this: an image is fetched
+    /// once and never seeks, so it never displaces the video being watched.
+    private var media: (key: String, mimeType: String?, data: Data)?
     private let lock = NSLock()
 
     func webView(_ webView: WKWebView, start task: any WKURLSchemeTask) {
@@ -1080,6 +1128,15 @@ final class GitHubAssetSchemeHandler: NSObject, WKURLSchemeHandler {
             return
         }
 
+        let seeking = task.request.value(forHTTPHeaderField: "Range") != nil
+        if seeking {
+            lock.lock(); let held = media?.key == raw ? media : nil; lock.unlock()
+            if let held {
+                settle { Self.respond(to: $0, url: url, mimeType: held.mimeType, body: held.data) }
+                return
+            }
+        }
+
         var request = URLRequest(url: url)
         if let host = url.host,
            host == "github.com" || host.hasSuffix(".githubusercontent.com"),
@@ -1093,16 +1150,16 @@ final class GitHubAssetSchemeHandler: NSObject, WKURLSchemeHandler {
         Task { @MainActor in
             do {
                 let (data, response) = try await URLSession.shared.data(for: request)
-                settle {
-                    // Frame the payload under the request's own custom-scheme URL so
-                    // WebKit doesn't reject a response that arrived over https.
-                    let framed = URLResponse(
-                        url: $0.request.url ?? url, mimeType: response.mimeType,
-                        expectedContentLength: data.count, textEncodingName: nil)
-                    $0.didReceive(framed)
-                    $0.didReceive(data)
-                    $0.didFinish()
+                // GitHub answers a refused asset with an HTML page, which would otherwise be
+                // held as the video and re-served for every seek that follows.
+                if let http = response as? HTTPURLResponse, http.statusCode >= 400 {
+                    settle { $0.didFailWithError(URLError(.badServerResponse)) }
+                    return
+                }
+                if seeking {
+                    self.lock.withLock { self.media = (key: raw, mimeType: response.mimeType, data: data) }
                 }
+                settle { Self.respond(to: $0, url: url, mimeType: response.mimeType, body: data) }
             } catch {
                 settle { $0.didFailWithError(error) }
             }
@@ -1112,6 +1169,54 @@ final class GitHubAssetSchemeHandler: NSObject, WKURLSchemeHandler {
     func webView(_ webView: WKWebView, stop task: any WKURLSchemeTask) {
         lock.lock(); live.remove(ObjectIdentifier(task)); lock.unlock()
     }
+
+    /// Drops the held asset — the page that was playing it is gone, and it is the largest
+    /// thing this handler keeps.
+    func forgetMedia() {
+        lock.lock(); media = nil; lock.unlock()
+    }
+
+    /// Answers under the custom-scheme URL — WebKit rejects a response that claims to have
+    /// arrived over https — serving the slice the task asked for. The `206` and its
+    /// `Content-Range` are what tell a `<video>` element it may seek; a plain `URLResponse`
+    /// carries neither status nor headers, and a range request answered with the whole file
+    /// reads as a broken stream.
+    private static func respond(to task: any WKURLSchemeTask, url: URL,
+                                mimeType: String?, body: Data) {
+        let target = task.request.url ?? url
+        let requested = byteRange(task.request.value(forHTTPHeaderField: "Range"), count: body.count)
+        let slice = requested.map { body.subdata(in: $0) } ?? body
+        var headers = [
+            "Content-Type": mimeType ?? "application/octet-stream",
+            "Content-Length": String(slice.count),
+            "Accept-Ranges": "bytes",
+        ]
+        if let requested {
+            headers["Content-Range"] =
+                "bytes \(requested.lowerBound)-\(requested.upperBound - 1)/\(body.count)"
+        }
+        let response = HTTPURLResponse(
+            url: target, statusCode: requested == nil ? 200 : 206,
+            httpVersion: "HTTP/1.1", headerFields: headers)
+            ?? URLResponse(url: target, mimeType: mimeType,
+                           expectedContentLength: slice.count, textEncodingName: nil)
+        task.didReceive(response)
+        task.didReceive(slice)
+        task.didFinish()
+    }
+
+    /// `bytes=first-last` with an open end, the only form WebKit's media loader sends.
+    /// Anything else (a suffix range, a multi-range list) answers as the whole asset, which
+    /// is a legal reply to any range request.
+    static func byteRange(_ header: String?, count: Int) -> Range<Int>? {
+        guard count > 0, let header, header.hasPrefix("bytes=") else { return nil }
+        let bounds = header.dropFirst("bytes=".count)
+            .split(separator: "-", omittingEmptySubsequences: false)
+        guard bounds.count == 2, let first = Int(bounds[0]), first < count else { return nil }
+        let last = Int(bounds[1]).map { min($0, count - 1) } ?? count - 1
+        guard last >= first else { return nil }
+        return first..<(last + 1)
+    }
 }
 
 private extension Date {
diff --git a/Tests/termioTests/IssueAttachmentTests.swift b/Tests/termioTests/IssueAttachmentTests.swift
new file mode 100644
index 00000000..cfc76d78
--- /dev/null
+++ b/Tests/termioTests/IssueAttachmentTests.swift
@@ -0,0 +1,56 @@
+import XCTest
+@testable import termio
+
+/// GitHub uploads a video as a bare attachment link, so the detail page has to recognize
+/// that shape and only that shape — every other bare link in a conversation is prose.
+final class IssueAttachmentTests: XCTestCase {
+    func testBareAttachmentLinkBecomesPlayer() {
+        let url = "https://github.com/user-attachments/assets/f65d6538-863f-40da-ae49-bea4d5fd15fd"
+        XCTAssertEqual(
+            IssueDetailHTML.embedAttachments("<p><a href=\"\(url)\">\(url)</a></p>"),
+            "<video class=\"attachment\" controls preload=\"metadata\" src=\"\(url)\"></video>"
+        )
+    }
+
+    func testLegacyUserImagesVideoBecomesPlayer() {
+        let url = "https://user-images.githubusercontent.com/1/demo.mp4"
+        XCTAssertTrue(
+            IssueDetailHTML.embedAttachments("<p><a href=\"\(url)\">\(url)</a></p>")
+                .hasPrefix("<video")
+        )
+    }
+
+    func testOrdinaryLinksAreLeftAlone() {
+        for url in [
+            "https://github.com/termio-sh/termio/issues/272",
+            "https://example.com/user-attachments/assets/1234",
+            "https://user-images.githubusercontent.com/1/shot.png",
+        ] {
+            let paragraph = "<p><a href=\"\(url)\">\(url)</a></p>"
+            XCTAssertEqual(IssueDetailHTML.embedAttachments(paragraph), paragraph)
+        }
+    }
+
+    func testLinkedAttachmentInsideProseStaysALink() {
+        let html = "<p>see <a href=\"https://github.com/user-attachments/assets/abc\">this</a> too</p>"
+        XCTAssertEqual(IssueDetailHTML.embedAttachments(html), html)
+    }
+
+    /// The byte ranges a `<video>` element asks the asset loader for: playback stalls on any
+    /// slice that comes back off by one, so each bound is pinned.
+    func testByteRangesTheMediaLoaderSends() {
+        XCTAssertEqual(GitHubAssetSchemeHandler.byteRange("bytes=0-1", count: 100), 0..<2)
+        XCTAssertEqual(GitHubAssetSchemeHandler.byteRange("bytes=10-", count: 100), 10..<100)
+        // An end past the last byte clamps rather than overruns the buffer.
+        XCTAssertEqual(GitHubAssetSchemeHandler.byteRange("bytes=90-200", count: 100), 90..<100)
+    }
+
+    /// Everything else answers as the whole asset — a legal reply to any range request.
+    func testUnrangedRequestsTakeTheWholeAsset() {
+        XCTAssertNil(GitHubAssetSchemeHandler.byteRange(nil, count: 100))
+        XCTAssertNil(GitHubAssetSchemeHandler.byteRange("bytes=-500", count: 100))
+        XCTAssertNil(GitHubAssetSchemeHandler.byteRange("bytes=0-1,5-6", count: 100))
+        XCTAssertNil(GitHubAssetSchemeHandler.byteRange("bytes=200-300", count: 100))
+        XCTAssertNil(GitHubAssetSchemeHandler.byteRange("bytes=0-1", count: 0))
+    }
+}
diff --git a/Tests/termioTests/MarkdownHTMLTests.swift b/Tests/termioTests/MarkdownHTMLTests.swift
index c3ea2085..9e401d87 100644
--- a/Tests/termioTests/MarkdownHTMLTests.swift
+++ b/Tests/termioTests/MarkdownHTMLTests.swift
@@ -25,6 +25,13 @@ final class HTMLSanitizerTests: XCTestCase {
         )
     }
 
+    func testVideoKeepsItsPlayerAttributesButNeverAutoplays() {
+        XCTAssertEqual(
+            HTMLSanitizer.sanitize(#"<video src="a.mp4" controls muted autoplay poster="p.png">"#),
+            #"<video src="a.mp4" controls muted poster="p.png">"#
+        )
+    }
+
     func testJavascriptURLsDropped() {
         XCTAssertEqual(HTMLSanitizer.sanitize(#"<a href="javascript:alert(1)">x</a>"#), "<a>x</a>")
         XCTAssertEqual(HTMLSanitizer.sanitize(#"<img src="data:text/html,x">"#), "<img>")