commit f7e407e030cfe34ae726d8c2509307bebc6b0c9a
parent e547e26d99536e1f48c346ba8ead87a5770f97e5
Author: Daniel D’Aquino <daniel@daquino.me>
Date: Fri, 22 Dec 2023 21:25:15 +0000
Fix crash on very large notes
This commit fixes a crash that occured on large notes with several artifacts.
The crash was caused by an empirically observed limitation on the amount
of `Text` objects that can be added together. Adding several `Text`
objects together causes a infinite recursion and subsequent stack
overflow.
The fix applied changes the `CompatibleText` class to store several
items in a list, which concatenates attributed strings when possible to
reducethe amount of `Text` objects used.
This commit also:
- Changes the structure to avoid storing SwiftUI objects on a variable,
but making it into a computed property instead.
- Renders a nice error message when the note is too large to be rendered
(instead of crashing)
With this new commit, we can render much larger notes, and the only ones
that will not be displayed are those containing more than 50 custom
hashtags.
Since we do not even have 50 custom hashtag types, the only notes that
won't be rendered are spammy notes that repeat the same hashtags over
and over again.
Testing
-------
PASS
Device: iPhone 13 Mini (Physical device)
iOS: 17.2
Damus: This commit
Setup:
- Local test relay and a test account running on a simulator to post
those long test notes.
- Local web page server to serve a link to the problematic note.
(nostr:note1ttfgneka3lt6yuutmr0uls5xd6z975fgdzpfkxwwf40dd38pjcqqvzvxaj)
Steps
-----
1. Click on the link to open the note
2. Check that no crash occurs and that the note is rendered correctly. PASS
3. Click on the note to render the SelectableText view (Different code
path). Make sure that no crash occurs and that it is rendered
correctly. PASS
4. On the simulator, post a note with 50 "#bitcoin" hashtags displayed
(100 items).
5. Open the note on the physical iPhone. Make sure that no crash occurs
and that the note is rendered correctly. PASS
6. Ensure that the hashtag and hashtag icons are rendered. PASS
7. Click on the note and make sure that the SelectableText view is
rendered correctly. PASS[1]
8. On the simulator, post a note with 51 "#bitcoin" hashtags displayed
(102 items).
9. Open the note on the physical iPhone. Make sure that a nice error
message is displayed. PASS
10. Click on the note and make sure that the SelectableText view is
rendered correctly. PASS[1]
Notes
[1] On the SelectableText view, special hashtags always render with the
purple color. This behavior was already present before the changes, so
it is not a regression.
Changelog-Fixed: Fix crash on very large notes
Closes: https://github.com/damus-io/damus/issues/1826
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Reviewed-by: William Casarin <jb55@jb55.com>
Signed-off-by: William Casarin <jb55@jb55.com>
Diffstat:
3 files changed, 94 insertions(+), 39 deletions(-)
diff --git a/damus/Util/CompatibleAttribute.swift b/damus/Util/CompatibleAttribute.swift
@@ -8,37 +8,96 @@
import Foundation
import SwiftUI
+// Concatening too many `Text` objects can cause crashes (See https://github.com/damus-io/damus/issues/1826)
+fileprivate let MAX_TEXT_ITEMS = 100
+
class CompatibleText: Equatable {
- var text: Text
- var attributed: AttributedString
-
+ var text: some View {
+ if items.count > MAX_TEXT_ITEMS {
+ return AnyView(
+ VStack {
+ Image("warning")
+ Text(NSLocalizedString("This note contains too many items and cannot be rendered", comment: "Error message indicating that a note is too big and cannot be rendered"))
+ .multilineTextAlignment(.center)
+ }
+ .foregroundColor(.secondary)
+ )
+ }
+ return AnyView(
+ items.reduce(Text(""), { (accumulated, item) in
+ return accumulated + item.render_to_text()
+ })
+ )
+ }
+ var attributed: AttributedString {
+ return items.reduce(AttributedString(stringLiteral: ""), { (accumulated, item) in
+ guard let item_attributed_string = item.attributed_string() else { return accumulated }
+ return accumulated + item_attributed_string
+ })
+ }
+ var items: [Item]
+
init() {
- self.text = Text("")
- self.attributed = AttributedString(stringLiteral: "")
+ self.items = [.attributed_string(AttributedString(stringLiteral: ""))]
}
-
+
init(stringLiteral: String) {
- self.text = Text(stringLiteral)
- self.attributed = AttributedString(stringLiteral: stringLiteral)
+ self.items = [.attributed_string(AttributedString(stringLiteral: stringLiteral))]
}
-
- init(text: Text, attributed: AttributedString) {
- self.text = text
- self.attributed = attributed
- }
-
+
init(attributed: AttributedString) {
- self.text = Text(attributed)
- self.attributed = attributed
+ self.items = [.attributed_string(attributed)]
+ }
+
+ init(items: [Item]) {
+ self.items = items
}
-
+
static func == (lhs: CompatibleText, rhs: CompatibleText) -> Bool {
- return lhs.attributed == rhs.attributed
+ return lhs.items == rhs.items
}
-
+
static func +(lhs: CompatibleText, rhs: CompatibleText) -> CompatibleText {
- let combinedText = lhs.text + rhs.text
- let combinedAttributes = lhs.attributed + rhs.attributed
- return CompatibleText(text: combinedText, attributed: combinedAttributes)
+ if case .attributed_string(let las) = lhs.items.last,
+ case .attributed_string(let ras) = rhs.items.first
+ {
+ // Concatenate attributed strings whenever possible to reduce item count
+ let combined_attributed_string = las + ras
+ return CompatibleText(items:
+ Array(lhs.items.prefix(upTo: lhs.items.count - 1)) +
+ [.attributed_string(combined_attributed_string)] +
+ Array(rhs.items.suffix(from: 1))
+ )
+ }
+ else {
+ return CompatibleText(items: lhs.items + rhs.items)
+ }
+ }
+
+}
+
+extension CompatibleText {
+ enum Item: Equatable {
+ case attributed_string(AttributedString)
+ case icon(named: String, offset: CGFloat)
+
+ func render_to_text() -> Text {
+ switch self {
+ case .attributed_string(let attributed_string):
+ return Text(attributed_string)
+ case .icon(named: let image_name, offset: let offset):
+ return Text(Image(image_name)).baselineOffset(offset)
+ }
+ }
+
+ func attributed_string() -> AttributedString? {
+ switch self {
+ case .attributed_string(let attributed_string):
+ return attributed_string
+ case .icon(named: let name, offset: _):
+ guard let img = UIImage(named: name) else { return nil }
+ return icon_attributed_string(img: img)
+ }
+ }
}
}
diff --git a/damus/Util/Hashtags.swift b/damus/Util/Hashtags.swift
@@ -43,28 +43,20 @@ let custom_hashtags: [String: CustomHashtag] = [
func hashtag_str(_ htag: String) -> CompatibleText {
var attributedString = AttributedString(stringLiteral: "#\(htag)")
attributedString.link = URL(string: "damus:t:\(htag.addingPercentEncoding(withAllowedCharacters: .urlHostAllowed) ?? htag)")
-
+
let lowertag = htag.lowercased()
-
- var text = Text(attributedString)
+
if let custom_hashtag = custom_hashtags[lowertag] {
if let col = custom_hashtag.color {
attributedString.foregroundColor = col
}
-
+
let name = custom_hashtag.name
-
- if let img = UIImage(named: "\(name)-hashtag") {
- attributedString = attributedString + " "
- attributed_string_attach_icon(&attributedString, img: img)
- }
- text = Text(attributedString)
- let img = Image("\(name)-hashtag")
- text = text + Text(img).baselineOffset(custom_hashtag.offset ?? 0.0)
+
+ attributedString = attributedString + " "
+ return CompatibleText(items: [.attributed_string(attributedString), .icon(named: "\(name)-hashtag", offset: custom_hashtag.offset ?? 0.0)])
} else {
attributedString.foregroundColor = DamusColors.purple
+ return CompatibleText(items: [.attributed_string(attributedString)])
}
-
- return CompatibleText(text: text, attributed: attributedString)
}
-
diff --git a/damus/Views/NoteContentView.swift b/damus/Views/NoteContentView.swift
@@ -297,11 +297,15 @@ struct NoteContentView: View {
}
func attributed_string_attach_icon(_ astr: inout AttributedString, img: UIImage) {
+ let wrapped = icon_attributed_string(img: img)
+ astr.append(wrapped)
+}
+
+func icon_attributed_string(img: UIImage) -> AttributedString {
let attachment = NSTextAttachment()
attachment.image = img
let attachmentString = NSAttributedString(attachment: attachment)
- let wrapped = AttributedString(attachmentString)
- astr.append(wrapped)
+ return AttributedString(attachmentString)
}
func url_str(_ url: URL) -> CompatibleText {
