From 35965ef250fb271070f7e1ead4372379d0233555 Mon Sep 17 00:00:00 2001 From: welshofer Date: Tue, 25 Aug 2026 10:27:01 -0700 Subject: [PATCH 1/2] Make rendered slide text visible in a browser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults made every deck render as a coloured rectangle with no readable text, which is what any WebKit-backed viewer shows — Lectern's own slide previews and contact sheet included. Font sizes went out in EMU, like every other length here. Browsers clamp a computed font-size to a five-digit maximum *before* the viewBox transform is applied, so a 68pt title asking for font-size="863600" was clamped and then scaled down to roughly one pixel: text present, correctly placed, invisible. Sizes now go out in points under a per-text translate(x,y) scale(12700), which keeps the number far below the clamp and puts the glyphs back in EMU space. text-anchor is unaffected — it anchors at x=0 of the scaled space, which the translate has already moved to the anchor point. The renderer also resolved each run's typeface to pick wrapping metrics but never named it in the markup, so a deck rendered in the viewer's default serif whatever its brand font was. Resolved typefaces are now emitted as font-family with a sans-serif fallback. All five text emission sites (body text, the unsupported-graphic placeholder, chart titles, category labels, legends) go through one textElement helper rather than repeating the attribute soup four more ways. Verified on a 49-slide Keynote-authored deck: previously 49 black rectangles, now fully legible in its own Helvetica Neue. 678 Rostrum tests and 162 Lectern tests pass. One existing assertion checked for the EMU size string and is updated to the points encoding; a second checked ` --- .../Rostrum/Presentation/SVGRenderer.swift | 93 +++++++++++++++---- Tests/RostrumTests/SVGRendererTests.swift | 60 +++++++++++- 2 files changed, 133 insertions(+), 20 deletions(-) diff --git a/Sources/Rostrum/Presentation/SVGRenderer.swift b/Sources/Rostrum/Presentation/SVGRenderer.swift index 8c938f99..4fdd3383 100644 --- a/Sources/Rostrum/Presentation/SVGRenderer.swift +++ b/Sources/Rostrum/Presentation/SVGRenderer.swift @@ -255,6 +255,7 @@ struct SVGRenderer { let x: Int, baseline: Int, size: Int let fill: String, anchor: String, text: String let bold: Bool + let typeface: String? } var lines: [Line] = [] var cursorY = 0 @@ -309,7 +310,8 @@ struct SVGRenderer { let ascent = Int((metrics.ascent(pointSize: sizePt) * Double(emuPerPoint)).rounded()) for line in wrapped { lines.append(Line(x: lineX, baseline: cursorY + ascent, size: sizeEMU, - fill: color, anchor: textAnchor, text: line, bold: bold)) + fill: color, anchor: textAnchor, text: line, bold: bold, + typeface: typeface)) cursorY += lineH } } else { @@ -320,7 +322,8 @@ struct SVGRenderer { for line in wrapEstimated(text, width: w, sizeEMU: sizeEMU) { cursorY += sizeEMU lines.append(Line(x: anchorX, baseline: cursorY, size: sizeEMU, - fill: color, anchor: textAnchor, text: line, bold: bold)) + fill: color, anchor: textAnchor, text: line, bold: bold, + typeface: typeface)) cursorY += sizeEMU / 3 } } @@ -337,13 +340,63 @@ struct SVGRenderer { default: offsetY = y } return lines.map { line in - "" - + escape(line.text) + "" + textElement(line.text, x: line.x, baseline: line.baseline + offsetY, + sizeEMU: line.size, fill: line.fill, anchor: line.anchor, + bold: line.bold, typeface: line.typeface) }.joined() } + + // MARK: - Text emission + + /// One ``, positioned in EMU but sized in points. + /// + /// The obvious markup — `font-size` in EMU, like every other length here — + /// is silently unreadable in a browser. WebKit and Blink clamp computed + /// `font-size` to a five-digit maximum *before* the viewBox transform + /// shrinks it, so a 68pt title asking for `font-size="863600"` gets clamped + /// and then scaled down to roughly one pixel. The text is present, in the + /// right place, and invisible. + /// + /// So the glyphs are specified in points, under their own + /// `translate(x, y) scale(emuPerPoint)`: the size never approaches the + /// clamp, and the scale puts it back into EMU space. `text-anchor` still + /// works — it anchors at x = 0 of the scaled space, which the translate has + /// already put at the anchor point. + private func textElement(_ text: String, x: Int, baseline: Int, sizeEMU: Int, + fill: String, anchor: String, bold: Bool, + typeface: String?) -> String { + "" + + escape(text) + "" + } + + /// The typeface the run resolved to, as a `font-family` the viewer can use. + /// + /// Without this every deck renders in the viewer's default serif, whatever + /// its brand font is — the renderer already resolves the typeface to pick + /// wrapping metrics, it just never said so in the markup. A generic + /// fallback keeps a missing font from landing back on serif by accident. + private func fontFamilyAttr(_ typeface: String?) -> String { + guard let typeface, !typeface.isEmpty else { return "" } + return " font-family=\"\(escape(typeface)), sans-serif\"" + } + + /// EMU as points, formatted deterministically. + /// + /// Run sizes come from `a:rPr/@sz` in hundredths of a point, so this is at + /// most two decimals; trailing zeros are trimmed so whole sizes stay whole + /// and byte-identical output survives. + private func points(_ emu: Int) -> String { + let hundredths = emu * 100 / emuPerPoint + let whole = hundredths / 100, frac = abs(hundredths % 100) + if frac == 0 { return String(whole) } + if frac % 10 == 0 { return "\(whole).\(frac / 10)" } + return String(format: "%d.%02d", whole, frac) + } + /// The typeface a run renders in: its own `a:latin`, the theme font it /// names indirectly (`+mj-lt`/`+mn-lt`), or — when it names none — the /// first theme font the deck has metrics for. @@ -500,8 +553,9 @@ struct SVGRenderer { else if uri == GraphicDataURI.ole { label = "[embedded object]" } else { label = "[object]" } return box(x, y, w, h, fill: "#F2F2F2", stroke: " stroke=\"#CCCCCC\" stroke-width=\"6350\"") - + "\(escape(label))" + + textElement(label, x: x + w / 2, baseline: y + h / 2, + sizeEMU: 18 * emuPerPoint, fill: "#999999", anchor: "middle", + bold: false, typeface: nil) } // MARK: - Charts @@ -550,9 +604,10 @@ struct SVGRenderer { var out = "" if let title { - out += "" - + escape(clipLabel(title, width: w, sizeEMU: 13 * emuPerPoint)) + "" + out += textElement(clipLabel(title, width: w, sizeEMU: 13 * emuPerPoint), + x: coord(fx + fw / 2), baseline: coord(fy + fh * 0.11), + sizeEMU: 13 * emuPerPoint, fill: "#666666", anchor: "middle", + bold: false, typeface: nil) } switch kind { @@ -776,9 +831,11 @@ struct SVGRenderer { let size = 10 * emuPerPoint var out = "" for (index, label) in categories.prefix(catCount).enumerated() { - out += "" + escape(clipLabel(label, width: coord(step), sizeEMU: size)) + "" + out += textElement(clipLabel(label, width: coord(step), sizeEMU: size), + x: coord(plotX + step * (Double(index) + 0.5)), + baseline: coord(baseY + height * 0.06), + sizeEMU: size, fill: "#808080", anchor: "middle", + bold: false, typeface: nil) } return out } @@ -796,9 +853,11 @@ struct SVGRenderer { let left = x + slot * Double(slotIndex) + slot * 0.1 out += box(coord(left), coord(y + height * 0.02), coord(swatch), coord(swatch), fill: seriesColor(entry.offset)) - out += "" - + escape(clipLabel(entry.element, width: coord(slot * 0.75), sizeEMU: size)) + "" + out += textElement(clipLabel(entry.element, width: coord(slot * 0.75), sizeEMU: size), + x: coord(left + swatch * 1.5), + baseline: coord(y + height * 0.02 + swatch * 0.85), + sizeEMU: size, fill: "#808080", anchor: "start", + bold: false, typeface: nil) } return out } diff --git a/Tests/RostrumTests/SVGRendererTests.swift b/Tests/RostrumTests/SVGRendererTests.swift index 01e0c0e9..2c97a9ca 100644 --- a/Tests/RostrumTests/SVGRendererTests.swift +++ b/Tests/RostrumTests/SVGRendererTests.swift @@ -355,9 +355,63 @@ import Testing #expect(svg.contains("Inherited")) #expect(svg.contains("#FF0000"), "the run fell back to the renderer's default colour") - #expect(svg.contains("\(80 * 12700)"), "the run fell back to the renderer's default size") + #expect(svg.contains("font-size=\"80\""), "the run fell back to the renderer's default size") // The layout puts ctrTitle at x=1524000; a shape rendered without - // inheritance lands at 0. - #expect(!svg.contains(" [String] { + guard let re = try? NSRegularExpression(pattern: pattern) else { return [] } + let ns = text as NSString + return re.matches(in: text, range: NSRange(location: 0, length: ns.length)) + .compactMap { $0.numberOfRanges > 1 ? ns.substring(with: $0.range(at: 1)) : nil } } } From 396e0ab8fdf6cca9cf4cc218a24b73ca8d6a1404 Mon Sep 17 00:00:00 2001 From: welshofer Date: Tue, 25 Aug 2026 11:28:54 -0700 Subject: [PATCH 2/2] Carry parts with no content type instead of rejecting the deck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[Content_Types].xml` is required to cover every part (OPC M1.2), so a package that breaks that rule is malformed and the reader threw. PowerPoint writes them anyway: deleting content can leave a `/[trash]/0000.dat` in the archive with neither an Override nor a matching Default, and PowerPoint reopens its own file without complaint. Refusing the whole deck over an entry nothing references cost 12 of 471 real decks in one library — 2.5%, every one of which opens in PowerPoint and in python-pptx. Measured, not guessed: the same corpus's other 13 failures are genuinely truncated archives that Python also refuses, and those still throw. No deck in 471 hit the zip64 path. So an untyped entry is now carried rather than rejected — the same treatment as an orphan `.rels` stream, and for the same reason. It cannot become a `Part`, since a `Part` with no content type has no legal serialization, and it cannot be dropped, since lossless round-tripping is this library's standing rule. It goes in `untypedEntries`, is re-emitted in sorted order beside the other carried entries, and is recorded in `readWarnings` rather than swallowed. The guard is "no declared content type", not "lives in [trash]": the rule is about the declaration, and other producers leave other names behind. `ContentTypesMap` gains a non-throwing `declaredContentType(for:)` so `read` can branch rather than catch. The throwing `contentType(for:)` is unchanged. 678 existing tests still pass, including the byte-identical round-trip corpus. Seven new tests cover carrying, the read warning, survival across a resave, resave being a fixed point, and that a declared extension is still a part. Co-Authored-By: Claude Opus 5 --- Sources/Rostrum/OPC/ContentTypes.swift | 14 +++- Sources/Rostrum/OPC/OPCPackage.swift | 31 +++++++- Tests/RostrumTests/UntypedEntryTests.swift | 83 ++++++++++++++++++++++ 3 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 Tests/RostrumTests/UntypedEntryTests.swift diff --git a/Sources/Rostrum/OPC/ContentTypes.swift b/Sources/Rostrum/OPC/ContentTypes.swift index 599e439a..ba07cdd6 100644 --- a/Sources/Rostrum/OPC/ContentTypes.swift +++ b/Sources/Rostrum/OPC/ContentTypes.swift @@ -44,11 +44,19 @@ public struct ContentTypesMap { overrides[partName] = nil } + /// Override first, then extension Default. Nil when the package declares + /// neither — which is malformed, but survivable; see + /// `OPCPackage.untypedEntries`. + public func declaredContentType(for partName: PackURI) -> String? { + overrides[partName] ?? defaults[partName.ext] + } + /// Override first, then extension Default. public func contentType(for partName: PackURI) throws -> String { - if let ct = overrides[partName] { return ct } - if let ct = defaults[partName.ext] { return ct } - throw RostrumError.packageInvalid("no content type for part \(partName)") + guard let ct = declaredContentType(for: partName) else { + throw RostrumError.packageInvalid("no content type for part \(partName)") + } + return ct } public static func parse(_ data: Data) throws -> ContentTypesMap { diff --git a/Sources/Rostrum/OPC/OPCPackage.swift b/Sources/Rostrum/OPC/OPCPackage.swift index a7098d91..3c379903 100644 --- a/Sources/Rostrum/OPC/OPCPackage.swift +++ b/Sources/Rostrum/OPC/OPCPackage.swift @@ -94,6 +94,22 @@ public final class OPCPackage { /// read, not modelled, and previously not written back. private(set) var orphanRelationshipStreams: [(name: String, data: Data)] = [] + /// Entries with no declared content type. + /// + /// `[Content_Types].xml` is required to cover every part (OPC M1.2), and a + /// package that breaks that rule is malformed. But PowerPoint itself ships + /// them: deleting content can leave a `/[trash]/0000.dat` behind in the + /// archive with no Override and no matching Default, and PowerPoint reopens + /// its own files perfectly happily. Refusing the whole deck over a part + /// nothing references cost 12 of 471 real decks in one library — 2.5%, + /// every one of which opens in PowerPoint and in python-pptx. + /// + /// So they are carried, not modelled and not rejected: same treatment as an + /// orphan `.rels` stream, and for the same reason. They cannot become + /// `Part`s — a `Part` without a content type has no legal serialization — + /// and dropping them would break the round trip. + private(set) var untypedEntries: [(name: String, data: Data)] = [] + /// Diagnostics from `read`: carried entries (directory placeholders, /// orphan `.rels` streams) that could not be decoded and were dropped. /// Opening must survive them — they are not parts, and failing the whole @@ -169,7 +185,16 @@ public final class OPCPackage { } let uri = PackURI("/" + name) let blob = try zip.data(forEntry: name) - let ct = try package.contentTypes.contentType(for: uri) + guard let ct = package.contentTypes.declaredContentType(for: uri) else { + // Not a part — nothing can reference it, because a relationship + // target without a content type could not be loaded either. + // Carried verbatim so the resave stays a fixed point. + package.untypedEntries.append((name, blob)) + package.readWarnings.append( + "part \"\(name)\" has no declared content type and is carried " + + "through unmodelled") + continue + } package.parts[uri] = Part(uri: uri, contentType: ct, blob: blob) } @@ -314,6 +339,10 @@ public final class OPCPackage { where !derived.contains(entry.name) { zip.addFile(name: entry.name, data: entry.data) } + for entry in untypedEntries.sorted(by: { $0.name < $1.name }) + where !derived.contains(entry.name) { + zip.addFile(name: entry.name, data: entry.data) + } for entry in directoryEntries.sorted(by: { $0.name < $1.name }) where !derived.contains(entry.name) { // Compress like any other entry. A placeholder is normally empty, diff --git a/Tests/RostrumTests/UntypedEntryTests.swift b/Tests/RostrumTests/UntypedEntryTests.swift new file mode 100644 index 00000000..9474c2bb --- /dev/null +++ b/Tests/RostrumTests/UntypedEntryTests.swift @@ -0,0 +1,83 @@ +import Foundation +import Testing +@testable import Rostrum + +/// A part with no declared content type is malformed by OPC M1.2 — and +/// PowerPoint writes them anyway. Deleting content can leave a `/[trash]/…` +/// entry in the archive with neither an Override nor a matching Default, and +/// PowerPoint reopens its own file without complaint. +/// +/// Rejecting the whole package over one cost 12 of 471 real decks in a single +/// library (2.5%), every one of which opens in PowerPoint and in python-pptx. +/// So they are carried rather than rejected — and carried means they survive a +/// resave, because losing them would trade one bug for a quieter one. +@Suite struct UntypedEntryTests { + /// A minimal deck plus one entry nothing declares a type for. + private func deckWithTrash(_ trashName: String = "[trash]/0000.dat", + payload: Data = Data([0xDE, 0xAD, 0xBE, 0xEF])) throws -> Data { + let original = try MinimalTemplate.makePackage().serialize() + let reader = try ZipReader(data: original) + + var zip = ZipWriter() + for name in reader.entryNames { + zip.addFile(name: name, data: try reader.data(forEntry: name)) + } + zip.addFile(name: trashName, data: payload) + return try zip.finalize() + } + + @Test func aPartWithNoContentTypeDoesNotSinkThePackage() throws { + let package = try OPCPackage.read(data: try deckWithTrash()) + + #expect(package.parts[PackURI("/[trash]/0000.dat")] == nil, + "an untyped entry must not become a Part — it has no legal serialization") + #expect(package.untypedEntries.contains { $0.name == "[trash]/0000.dat" }) + } + + @Test func theDeckItselfOpensAndIsUsable() throws { + let deck = try Presentation(data: try deckWithTrash()) + #expect(deck.slides.count == 1, "the real content survived the malformed entry") + } + + /// Silence here would be the worse bug: the package is malformed, the + /// reader coped, and a caller checking round-trip fidelity deserves to know. + @Test func carryingIsRecordedNotSilent() throws { + let deck = try Presentation(data: try deckWithTrash()) + #expect(deck.readWarnings.contains { $0.contains("[trash]/0000.dat") }) + } + + /// Lossless round-tripping is the library's standing rule, and a carried + /// entry is exactly the kind of thing a resave quietly drops. + @Test func aCarriedEntrySurvivesAResave() throws { + let payload = Data("not a part".utf8) + let package = try OPCPackage.read(data: try deckWithTrash(payload: payload)) + let resaved = try OPCPackage.read(data: try package.serialize()) + + let carried = resaved.untypedEntries.first { $0.name == "[trash]/0000.dat" } + #expect(carried?.data == payload, "the bytes came back changed or not at all") + } + + @Test func resavingTwiceIsAFixedPoint() throws { + let once = try OPCPackage.read(data: try deckWithTrash()).serialize() + let twice = try OPCPackage.read(data: once).serialize() + #expect(once == twice, "determinism must survive the carried entry") + } + + /// The guard is "no declared content type", not "lives in [trash]" — the + /// rule is about the declaration, and other producers leave other names. + @Test func theRuleIsAboutTheDeclarationNotTheName() throws { + let package = try OPCPackage.read( + data: try deckWithTrash("ppt/leftovers/stray.bin")) + #expect(package.untypedEntries.contains { $0.name == "ppt/leftovers/stray.bin" }) + } + + /// An entry whose extension *is* declared is still a real part; carrying + /// must not become a way for content to go missing. + @Test func aDeclaredExtensionIsStillAPart() throws { + let package = try OPCPackage.read(data: try deckWithTrash("ppt/extra.xml")) + + #expect(package.parts[PackURI("/ppt/extra.xml")] != nil, + "the Default for \"xml\" covers this — it is a part, not a stray") + #expect(!package.untypedEntries.contains { $0.name == "ppt/extra.xml" }) + } +}