From 2885f6e8de3db160e7acdd5ae9487c3b65f9ab0b Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Mon, 10 Nov 2025 20:48:35 +1100 Subject: [PATCH 01/13] Forum Attachment Support - New option added "Forum Attachment". - Menu Tree updated to handle new option. Now [img] will prompt: a) (If clipboard not empty) Paste URL b) From Library (from Camera has been removed) If From Library is selected, user can select their image. Once selected, another pair of options is presented: a) Image Host (if imgurUploadsEnabled) b) Forum Attachment - If image is too large (dimensions or filesize) (likely!) then option to Resize & Continue is presented. User can cancel out of resize to meet reqs. - Preview card UI element appears during resizing and also to display attachment thumbnail, dimensions and filesize in newreply view - Preview card with Keep or Delete segmented options present when editing a post with an attachment - Fixed existing issue where attachments would not render in posts. Had to download these with authentication and then display them in the posts after the html renders --- App/Composition/AttachmentEditView.swift | 135 ++++++++++ App/Composition/AttachmentPreviewView.swift | 128 ++++++++++ App/Composition/CompositionMenuTree.swift | 230 +++++++++++++++--- .../CompositionViewController.swift | 216 ++++++++++++++-- App/Composition/ForumAttachment.swift | 192 +++++++++++++++ App/Misc/HTMLRenderingHelpers.swift | 44 +++- App/Posts/ReplyWorkspace.swift | 112 +++++++-- App/Resources/RenderView.js | 24 ++ .../Posts/PostsPageViewController.swift | 29 +++ App/Views/RenderView.swift | 41 +++- Awful.xcodeproj/project.pbxproj | 14 +- .../AwfulCore/Networking/ForumsClient.swift | 152 +++++++++++- .../URLRequest+MultipartFormData.swift | 38 +++ 13 files changed, 1267 insertions(+), 88 deletions(-) create mode 100644 App/Composition/AttachmentEditView.swift create mode 100644 App/Composition/AttachmentPreviewView.swift create mode 100644 App/Composition/ForumAttachment.swift diff --git a/App/Composition/AttachmentEditView.swift b/App/Composition/AttachmentEditView.swift new file mode 100644 index 000000000..3b3036d7c --- /dev/null +++ b/App/Composition/AttachmentEditView.swift @@ -0,0 +1,135 @@ +// AttachmentEditView.swift +// +// Copyright 2025 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app + +import UIKit + +/// A card-style view that shows existing attachment info with options to keep or delete it. +final class AttachmentEditView: UIView { + + private let imageView: UIImageView = { + let iv = UIImageView() + iv.contentMode = .scaleAspectFit + iv.clipsToBounds = true + iv.layer.cornerRadius = 4 + iv.backgroundColor = .secondarySystemFill + iv.translatesAutoresizingMaskIntoConstraints = false + return iv + }() + + private let titleLabel: UILabel = { + let label = UILabel() + label.font = UIFont.preferredFont(forTextStyle: .subheadline) + label.text = "Current Attachment" + label.translatesAutoresizingMaskIntoConstraints = false + return label + }() + + private let detailLabel: UILabel = { + let label = UILabel() + label.font = UIFont.preferredFont(forTextStyle: .caption1) + label.textColor = .secondaryLabel + label.translatesAutoresizingMaskIntoConstraints = false + return label + }() + + private let actionSegmentedControl: UISegmentedControl = { + let items = ["Keep", "Delete"] + let sc = UISegmentedControl(items: items) + sc.selectedSegmentIndex = 0 + sc.translatesAutoresizingMaskIntoConstraints = false + return sc + }() + + var onActionChanged: ((AttachmentAction) -> Void)? + + func updateTextColor(_ color: UIColor?) { + titleLabel.textColor = color + detailLabel.textColor = color?.withAlphaComponent(0.7) + } + + func updateSegmentedControlColors(selectedColor: UIColor?) { + // Set normal state text color to white + actionSegmentedControl.setTitleTextAttributes([.foregroundColor: UIColor.white], for: .normal) + + // Set selected state text color to white with the selected background color + if let selectedColor = selectedColor { + actionSegmentedControl.setTitleTextAttributes([.foregroundColor: UIColor.white], for: .selected) + actionSegmentedControl.selectedSegmentTintColor = selectedColor + } + } + + enum AttachmentAction { + case keep + case delete + } + + init() { + super.init(frame: .zero) + setupViews() + } + + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + private func setupViews() { + // Background color will be set by the parent view controller based on theme + layer.cornerRadius = 8 + + addSubview(imageView) + addSubview(titleLabel) + addSubview(detailLabel) + addSubview(actionSegmentedControl) + + actionSegmentedControl.addTarget(self, action: #selector(actionChanged), for: .valueChanged) + + NSLayoutConstraint.activate([ + // Image view on the left + imageView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 12), + imageView.topAnchor.constraint(equalTo: topAnchor, constant: 12), + imageView.widthAnchor.constraint(equalToConstant: 60), + imageView.heightAnchor.constraint(equalToConstant: 60), + + // Title label + titleLabel.leadingAnchor.constraint(equalTo: imageView.trailingAnchor, constant: 12), + titleLabel.topAnchor.constraint(equalTo: topAnchor, constant: 16), + titleLabel.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -12), + + // Detail label below title + detailLabel.leadingAnchor.constraint(equalTo: titleLabel.leadingAnchor), + detailLabel.topAnchor.constraint(equalTo: titleLabel.bottomAnchor, constant: 4), + detailLabel.trailingAnchor.constraint(equalTo: titleLabel.trailingAnchor), + + // Segmented control at the bottom + actionSegmentedControl.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 12), + actionSegmentedControl.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -12), + actionSegmentedControl.topAnchor.constraint(equalTo: imageView.bottomAnchor, constant: 12), + actionSegmentedControl.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -12) + ]) + } + + @objc private func actionChanged() { + let action: AttachmentAction = actionSegmentedControl.selectedSegmentIndex == 0 ? .keep : .delete + onActionChanged?(action) + } + + func configure(filename: String, filesize: String?) { + titleLabel.text = "Current Attachment" + if let filesize = filesize { + detailLabel.text = "\(filename) • \(filesize)" + } else { + detailLabel.text = filename + } + + // Show a generic document icon since we don't have the actual image + let config = UIImage.SymbolConfiguration(pointSize: 40, weight: .light) + imageView.image = UIImage(systemName: "doc.fill", withConfiguration: config) + imageView.tintColor = .tertiaryLabel + imageView.contentMode = .center + } + + var selectedAction: AttachmentAction { + return actionSegmentedControl.selectedSegmentIndex == 0 ? .keep : .delete + } +} diff --git a/App/Composition/AttachmentPreviewView.swift b/App/Composition/AttachmentPreviewView.swift new file mode 100644 index 000000000..315ef715f --- /dev/null +++ b/App/Composition/AttachmentPreviewView.swift @@ -0,0 +1,128 @@ +// AttachmentPreviewView.swift +// +// Copyright 2025 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app + +import UIKit + +/// A card-style view that shows a preview of an attached image with options to remove it. +final class AttachmentPreviewView: UIView { + + private let imageView: UIImageView = { + let iv = UIImageView() + iv.contentMode = .scaleAspectFill + iv.clipsToBounds = true + iv.layer.cornerRadius = 4 + iv.translatesAutoresizingMaskIntoConstraints = false + return iv + }() + + private let titleLabel: UILabel = { + let label = UILabel() + label.font = UIFont.preferredFont(forTextStyle: .subheadline) + label.text = "Attachment" + label.translatesAutoresizingMaskIntoConstraints = false + return label + }() + + private let detailLabel: UILabel = { + let label = UILabel() + label.font = UIFont.preferredFont(forTextStyle: .caption1) + label.textColor = .secondaryLabel + label.translatesAutoresizingMaskIntoConstraints = false + return label + }() + + private let removeButton: UIButton = { + let button = UIButton(type: .system) + let config = UIImage.SymbolConfiguration(pointSize: 20, weight: .medium) + button.setImage(UIImage(systemName: "xmark.circle.fill", withConfiguration: config), for: .normal) + button.tintColor = .secondaryLabel + button.translatesAutoresizingMaskIntoConstraints = false + return button + }() + + var onRemove: (() -> Void)? + + func updateTextColor(_ color: UIColor?) { + titleLabel.textColor = color + detailLabel.textColor = color?.withAlphaComponent(0.7) + } + + func showResizingPlaceholder() { + titleLabel.text = "Resizing Image..." + detailLabel.text = "Please wait" + imageView.image = nil + imageView.backgroundColor = .secondarySystemFill + } + + init() { + super.init(frame: .zero) + setupViews() + } + + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + private func setupViews() { + // Background color will be set by the parent view controller based on theme + layer.cornerRadius = 8 + + addSubview(imageView) + addSubview(titleLabel) + addSubview(detailLabel) + addSubview(removeButton) + + removeButton.addTarget(self, action: #selector(didTapRemove), for: .touchUpInside) + + NSLayoutConstraint.activate([ + // Image view on the left + imageView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 12), + imageView.topAnchor.constraint(equalTo: topAnchor, constant: 12), + imageView.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -12), + imageView.widthAnchor.constraint(equalToConstant: 60), + imageView.heightAnchor.constraint(equalToConstant: 60), + + // Title label + titleLabel.leadingAnchor.constraint(equalTo: imageView.trailingAnchor, constant: 12), + titleLabel.topAnchor.constraint(equalTo: topAnchor, constant: 16), + titleLabel.trailingAnchor.constraint(equalTo: removeButton.leadingAnchor, constant: -8), + + // Detail label below title + detailLabel.leadingAnchor.constraint(equalTo: titleLabel.leadingAnchor), + detailLabel.topAnchor.constraint(equalTo: titleLabel.bottomAnchor, constant: 4), + detailLabel.trailingAnchor.constraint(equalTo: titleLabel.trailingAnchor), + + // Remove button on the right + removeButton.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -12), + removeButton.centerYAnchor.constraint(equalTo: centerYAnchor), + removeButton.widthAnchor.constraint(equalToConstant: 30), + removeButton.heightAnchor.constraint(equalToConstant: 30) + ]) + } + + @objc private func didTapRemove() { + onRemove?() + } + + func configure(with attachment: ForumAttachment) { + titleLabel.text = "Attachment" + imageView.backgroundColor = .clear + imageView.image = attachment.image + + if let image = attachment.image { + let width = Int(image.size.width * image.scale) + let height = Int(image.size.height * image.scale) + + do { + let (data, _, _) = try attachment.imageData() + let formatter = ByteCountFormatter() + formatter.countStyle = .file + let sizeString = formatter.string(fromByteCount: Int64(data.count)) + detailLabel.text = "\(width) × \(height) • \(sizeString)" + } catch { + detailLabel.text = "\(width) × \(height)" + } + } + } +} diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 3ecacfb76..22bb21c8b 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -16,20 +16,27 @@ private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: /// Can take over UIMenuController to show a tree of composition-related items on behalf of a text view. final class CompositionMenuTree: NSObject { // This class exists to expose the struct-defined menu to Objective-C and to act as an image picker delegate. - + @FoilDefaultStorage(Settings.imgurUploadMode) private var imgurUploadMode - + fileprivate var imgurUploadsEnabled: Bool { return imgurUploadMode != .off } - + let textView: UITextView - + weak var draft: (NSObject & ReplyDraft)? + var onAttachmentChanged: (() -> Void)? + var onResizingStarted: (() -> Void)? + + private var pendingImage: UIImage? + private var pendingImageAssetIdentifier: String? + /// The textView's class will have some responder chain methods swizzled. - init(textView: UITextView) { + init(textView: UITextView, draft: (NSObject & ReplyDraft)? = nil) { self.textView = textView + self.draft = draft super.init() - + PSMenuItem.installMenuHandler(for: textView) NotificationCenter.default.addObserver(self, selector: #selector(UITextViewDelegate.textViewDidBeginEditing(_:)), name: UITextView.textDidBeginEditingNotification, object: textView) @@ -83,13 +90,7 @@ final class CompositionMenuTree: NSObject { shouldPopWhenMenuHides = true } - func showImagePicker(_ sourceType: UIImagePickerController.SourceType) { - // Check if we need to authenticate with Imgur first - if ImgurAuthManager.shared.needsAuthentication { - authenticateWithImgur() - return - } - + fileprivate func showImagePicker(_ sourceType: UIImagePickerController.SourceType) { let picker = UIImagePickerController() picker.sourceType = sourceType let mediaType = UTType.image @@ -269,16 +270,16 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont textView.nearestViewController?.present(alert, animated: true) return } - - if let asset = info[.phAsset] as? PHAsset { - insertImage(image, withAssetIdentifier: asset.localIdentifier) - } else { - insertImage(image) - } - picker.dismiss(animated: true, completion: { + let assetIdentifier = (info[.phAsset] as? PHAsset)?.localIdentifier + + pendingImage = image + pendingImageAssetIdentifier = assetIdentifier + + picker.dismiss(animated: true) { self.textView.becomeFirstResponder() - }) + self.showSubmenu(imageDestinationItems(tree: self)) + } } func imagePickerControllerDidCancel(_ picker: UIImagePickerController) { @@ -290,6 +291,141 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont func popoverPresentationControllerDidDismissPopover(_ popoverPresentationController: UIPopoverPresentationController) { textView.becomeFirstResponder() } + + private func validationErrorMessage(for error: ForumAttachment.ValidationError) -> String { + switch error { + case .fileTooLarge(let actualSize, let maxSize): + let formatter = ByteCountFormatter() + formatter.countStyle = .file + return "File size (\(formatter.string(fromByteCount: Int64(actualSize)))) exceeds maximum (\(formatter.string(fromByteCount: Int64(maxSize))))" + case .dimensionsTooLarge(let width, let height, let maxDimension): + return "Image dimensions (\(width)x\(height)) exceed maximum (\(maxDimension)x\(maxDimension))" + case .unsupportedFormat: + return "Unsupported image format. Please use GIF, JPG, JPEG, or PNG" + case .imageDataConversionFailed: + return "Failed to process image data" + } + } + + fileprivate func useImageHostForPendingImage() { + guard let image = pendingImage else { return } + + if ImgurAuthManager.shared.needsAuthentication { + authenticateWithImgur() + return + } + + if let assetID = pendingImageAssetIdentifier { + insertImage(image, withAssetIdentifier: assetID) + } else { + insertImage(image) + } + + pendingImage = nil + pendingImageAssetIdentifier = nil + } + + fileprivate func useForumAttachmentForPendingImage() { + guard let image = pendingImage else { return } + + let attachment = ForumAttachment(image: image, photoAssetIdentifier: pendingImageAssetIdentifier) + if let error = attachment.validationError { + if canResize(error) { + let alert = UIAlertController( + title: "Attachment Too Large", + message: "\(validationErrorMessage(for: error))\n\nWould you like to automatically resize the image to fit?", + preferredStyle: .alert + ) + alert.addAction(UIAlertAction(title: "Cancel", style: .cancel) { [weak self] _ in + self?.pendingImage = nil + self?.pendingImageAssetIdentifier = nil + }) + alert.addAction(UIAlertAction(title: "Resize & Continue", style: .default) { [weak self] _ in + self?.resizeAndAttachPendingImage() + }) + textView.nearestViewController?.present(alert, animated: true) + } else { + let alert = UIAlertController( + title: "Invalid Attachment", + message: validationErrorMessage(for: error), + alertActions: [.ok()] + ) + textView.nearestViewController?.present(alert, animated: true) + pendingImage = nil + pendingImageAssetIdentifier = nil + } + return + } + + draft?.forumAttachment = attachment + pendingImage = nil + pendingImageAssetIdentifier = nil + + onAttachmentChanged?() + // Success message removed - the preview card is sufficient feedback + } + + private func canResize(_ error: ForumAttachment.ValidationError) -> Bool { + switch error { + case .fileTooLarge, .dimensionsTooLarge: + return true + default: + return false + } + } + + private func resizeAndAttachPendingImage() { + guard let image = pendingImage else { return } + + // Show resizing placeholder immediately + onResizingStarted?() + + // Perform resize in background + DispatchQueue.global(qos: .userInitiated).async { [weak self] in + guard let self = self else { return } + + let originalAttachment = ForumAttachment(image: image, photoAssetIdentifier: self.pendingImageAssetIdentifier) + let resizedAttachment = originalAttachment.resized() + + DispatchQueue.main.async { + self.pendingImage = nil + self.pendingImageAssetIdentifier = nil + + guard let resizedAttachment = resizedAttachment else { + // Remove placeholder on failure + self.draft?.forumAttachment = nil + self.onAttachmentChanged?() + + let alert = UIAlertController( + title: "Resize Failed", + message: "Unable to resize image to meet requirements. Please try a different image.", + alertActions: [.ok()] + ) + self.textView.nearestViewController?.present(alert, animated: true) + return + } + + if let error = resizedAttachment.validationError { + // Remove placeholder on validation failure + self.draft?.forumAttachment = nil + self.onAttachmentChanged?() + + let alert = UIAlertController( + title: "Resize Failed", + message: "\(self.validationErrorMessage(for: error))\n\nPlease try a different image.", + alertActions: [.ok()] + ) + self.textView.nearestViewController?.present(alert, animated: true) + return + } + + // Update with resized attachment + self.draft?.forumAttachment = resizedAttachment + self.onAttachmentChanged?() + // Success message removed - the preview card is sufficient feedback + } + } + } } @objc protocol CompositionHidesMenuItems { @@ -331,10 +467,11 @@ fileprivate let rootItems = [ original line: MenuItem(title: "[img]", action: { $0.showSubmenu(imageItems) }), */ MenuItem(title: "[img]", action: { tree in - // If Imgur uploads are enabled in settings, show the full image submenu - // Otherwise, only allow pasting URLs - if tree.imgurUploadsEnabled { - tree.showSubmenu(imageItems) + // Show the image submenu if either: + // 1. Imgur uploads are enabled in settings, OR + // 2. The draft is a NewReplyDraft (for forum attachments) + if tree.imgurUploadsEnabled || tree.draft is NewReplyDraft { + tree.showSubmenu(imageItems(tree: tree)) } else { if UIPasteboard.general.coercedURL == nil { linkifySelection(tree) @@ -365,23 +502,38 @@ fileprivate let URLItems = [ }) ] -fileprivate let imageItems = [ - MenuItem(title: "From Camera", action: { $0.showImagePicker(.camera) }, enabled: isPickerAvailable(.camera)), - MenuItem(title: "From Library", action: { $0.showImagePicker(.photoLibrary) }, enabled: isPickerAvailable(.photoLibrary)), - MenuItem(title: "[img]", action: wrapSelectionInTag("[img]")), - MenuItem(title: "Paste [img]", action:{ tree in - if let text = UIPasteboard.general.coercedURL { +fileprivate func imageItems(tree: CompositionMenuTree) -> [MenuItem] { + var items: [MenuItem] = [] + + if UIPasteboard.general.coercedURL != nil { + items.append(MenuItem(title: "Paste URL", action: { tree in if let textRange = tree.textView.selectedTextRange { tree.textView.replace(textRange, withText:("[img]" + UIPasteboard.general.coercedURL!.absoluteString + "[/img]")) } - } - }, enabled: { UIPasteboard.general.coercedURL != nil }), - MenuItem(title: "Paste", action: { tree in - if let image = UIPasteboard.general.image { - tree.insertImage(image) - } - }, enabled: { UIPasteboard.general.image != nil }) -] + })) + } + + items.append(contentsOf: [ + MenuItem(title: "From Library", action: { $0.showImagePicker(.photoLibrary) }, enabled: isPickerAvailable(.photoLibrary)), + MenuItem(title: "[img]", action: wrapSelectionInTag("[img]")) + ]) + + return items +} + +fileprivate func imageDestinationItems(tree: CompositionMenuTree) -> [MenuItem] { + var items: [MenuItem] = [] + + if tree.imgurUploadsEnabled { + items.append(MenuItem(title: "Image Host", action: { $0.useImageHostForPendingImage() })) + } + + if tree.draft is NewReplyDraft { + items.append(MenuItem(title: "Forum Attachment", action: { $0.useForumAttachmentForPendingImage() })) + } + + return items +} fileprivate let formattingItems = [ MenuItem(title: "[b]", action: wrapSelectionInTag("[b]")), diff --git a/App/Composition/CompositionViewController.swift b/App/Composition/CompositionViewController.swift index 93f0d23ae..844aa1eeb 100644 --- a/App/Composition/CompositionViewController.swift +++ b/App/Composition/CompositionViewController.swift @@ -14,60 +14,242 @@ final class CompositionViewController: ViewController { super.init(nibName: nil, bundle: nil) restorationClass = type(of: self) } - + required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } - + override var title: String? { didSet { navigationItem.titleLabel.text = title navigationItem.titleLabel.sizeToFit() } } - + @objc fileprivate func didTapCancel() { if enableHaptics { UIImpactFeedbackGenerator(style: .medium).impactOccurred() } dismiss(animated: true) } - + @objc func cancel(_ sender: UIKeyCommand) { self.didTapCancel() } - + + private var _textView: CompositionTextView! var textView: UITextView { - return view as! UITextView + return _textView } - + + private let containerView = UIView() + private let attachmentPreviewView = AttachmentPreviewView() + private let attachmentEditView = AttachmentEditView() + private var attachmentPreviewHeightConstraint: NSLayoutConstraint! + private var attachmentEditHeightConstraint: NSLayoutConstraint! + private var textViewTopConstraint: NSLayoutConstraint! + override func loadView() { - let textView = CompositionTextView() - textView.restorationIdentifier = "Composition text view" - view = textView - - BBcodeBar = CompositionInputAccessoryView(textView: textView) - textView.inputAccessoryView = BBcodeBar + view = containerView + + _textView = CompositionTextView() + _textView.restorationIdentifier = "Composition text view" + _textView.translatesAutoresizingMaskIntoConstraints = false + + attachmentPreviewView.translatesAutoresizingMaskIntoConstraints = false + attachmentPreviewView.isHidden = true + attachmentPreviewView.onRemove = { [weak self] in + self?.removeAttachment() + } + + attachmentEditView.translatesAutoresizingMaskIntoConstraints = false + attachmentEditView.isHidden = true + attachmentEditView.onActionChanged = { [weak self] action in + self?.handleAttachmentEditAction(action) + } + + containerView.addSubview(attachmentPreviewView) + containerView.addSubview(attachmentEditView) + containerView.addSubview(_textView) + + attachmentPreviewHeightConstraint = attachmentPreviewView.heightAnchor.constraint(equalToConstant: 0) + attachmentEditHeightConstraint = attachmentEditView.heightAnchor.constraint(equalToConstant: 0) + + // Default: text view top anchors to preview view bottom (which starts at height 0) + textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) + + NSLayoutConstraint.activate([ + attachmentPreviewView.topAnchor.constraint(equalTo: containerView.safeAreaLayoutGuide.topAnchor, constant: 8), + attachmentPreviewView.leadingAnchor.constraint(equalTo: containerView.leadingAnchor, constant: 12), + attachmentPreviewView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor, constant: -12), + attachmentPreviewHeightConstraint, + + attachmentEditView.topAnchor.constraint(equalTo: containerView.safeAreaLayoutGuide.topAnchor, constant: 8), + attachmentEditView.leadingAnchor.constraint(equalTo: containerView.leadingAnchor, constant: 12), + attachmentEditView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor, constant: -12), + attachmentEditHeightConstraint, + + textViewTopConstraint, + _textView.leadingAnchor.constraint(equalTo: containerView.leadingAnchor), + _textView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor), + _textView.bottomAnchor.constraint(equalTo: containerView.bottomAnchor) + ]) + + BBcodeBar = CompositionInputAccessoryView(textView: _textView) + _textView.inputAccessoryView = BBcodeBar } - + fileprivate var keyboardAvoider: ScrollViewKeyboardAvoider? fileprivate var BBcodeBar: CompositionInputAccessoryView? fileprivate var menuTree: CompositionMenuTree? - + private weak var currentDraft: (NSObject & ReplyDraft)? + + func setDraft(_ draft: NSObject & ReplyDraft) { + menuTree?.draft = draft + currentDraft = draft + updateAttachmentPreview() + } + + func showResizingPlaceholder() { + attachmentPreviewView.showResizingPlaceholder() + attachmentPreviewView.isHidden = false + attachmentPreviewHeightConstraint.constant = 84 + + attachmentEditView.isHidden = true + attachmentEditHeightConstraint.constant = 0 + + // Update text view constraint to anchor to preview view + textViewTopConstraint.isActive = false + textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) + textViewTopConstraint.isActive = true + + UIView.animate(withDuration: 0.3) { + self.view.layoutIfNeeded() + } + } + + private func updateAttachmentPreview() { + guard let draft = currentDraft else { + hideAllAttachmentViews() + return + } + + // For edits, show existing attachment info if available + if let editDraft = draft as? EditReplyDraft { + if let existingFilename = editDraft.existingAttachmentFilename { + showAttachmentEditView(filename: existingFilename, filesize: editDraft.existingAttachmentFilesize) + return + } + } + + // For new posts, show preview if attachment is set + if let attachment = draft.forumAttachment { + showAttachmentPreview(with: attachment) + } else { + hideAllAttachmentViews() + } + } + + private func showAttachmentPreview(with attachment: ForumAttachment) { + attachmentEditView.isHidden = true + attachmentEditHeightConstraint.constant = 0 + + attachmentPreviewView.configure(with: attachment) + attachmentPreviewView.isHidden = false + attachmentPreviewHeightConstraint.constant = 84 + + // Update text view constraint to anchor to preview view + textViewTopConstraint.isActive = false + textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) + textViewTopConstraint.isActive = true + + UIView.animate(withDuration: 0.3) { + self.view.layoutIfNeeded() + } + } + + private func showAttachmentEditView(filename: String, filesize: String?) { + attachmentPreviewView.isHidden = true + attachmentPreviewHeightConstraint.constant = 0 + + attachmentEditView.configure(filename: filename, filesize: filesize) + attachmentEditView.isHidden = false + attachmentEditHeightConstraint.constant = 120 + + // Update text view constraint to anchor to edit view + textViewTopConstraint.isActive = false + textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentEditView.bottomAnchor, constant: 8) + textViewTopConstraint.isActive = true + + UIView.animate(withDuration: 0.3) { + self.view.layoutIfNeeded() + } + } + + private func hideAllAttachmentViews() { + attachmentPreviewView.isHidden = true + attachmentPreviewHeightConstraint.constant = 0 + + attachmentEditView.isHidden = true + attachmentEditHeightConstraint.constant = 0 + + // Reset text view constraint to anchor to preview view (at height 0) + textViewTopConstraint.isActive = false + textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) + textViewTopConstraint.isActive = true + + UIView.animate(withDuration: 0.3) { + self.view.layoutIfNeeded() + } + } + + private func removeAttachment() { + currentDraft?.forumAttachment = nil + hideAllAttachmentViews() + } + + private func handleAttachmentEditAction(_ action: AttachmentEditView.AttachmentAction) { + guard let editDraft = currentDraft as? EditReplyDraft else { return } + + editDraft.attachmentAction = (action == .keep) ? .keep : .delete + } + override func viewDidLoad() { super.viewDidLoad() - + keyboardAvoider = ScrollViewKeyboardAvoider(textView) menuTree = CompositionMenuTree(textView: textView) + menuTree?.onAttachmentChanged = { [weak self] in + self?.updateAttachmentPreview() + } + menuTree?.onResizingStarted = { [weak self] in + self?.showResizingPlaceholder() + } } override func themeDidChange() { super.themeDidChange() - + + textView.backgroundColor = theme["backgroundColor"] textView.textColor = theme["listTextColor"] textView.font = UIFont.preferredFontForTextStyle(.body, sizeAdjustment: -0.5, weight: .regular) textView.keyboardAppearance = theme.keyboardAppearance BBcodeBar?.keyboardAppearance = theme.keyboardAppearance + + // Theme the attachment cards + let listTextColor: UIColor? = theme["listTextColor"] + let borderColor: UIColor? = theme["listSecondaryTextColor"] + + attachmentPreviewView.backgroundColor = theme["backgroundColor"] + attachmentPreviewView.layer.borderColor = borderColor?.cgColor + attachmentPreviewView.layer.borderWidth = 1 + attachmentPreviewView.updateTextColor(listTextColor) + + attachmentEditView.backgroundColor = theme["backgroundColor"] + attachmentEditView.layer.borderColor = borderColor?.cgColor + attachmentEditView.layer.borderWidth = 1 + attachmentEditView.updateTextColor(listTextColor) + attachmentEditView.updateSegmentedControlColors(selectedColor: theme["tabBarIconSelectedColor"]) } override func viewWillAppear(_ animated: Bool) { diff --git a/App/Composition/ForumAttachment.swift b/App/Composition/ForumAttachment.swift new file mode 100644 index 000000000..1bb4a1425 --- /dev/null +++ b/App/Composition/ForumAttachment.swift @@ -0,0 +1,192 @@ +// ForumAttachment.swift +// +// Copyright 2025 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app + +import Photos +import UIKit + +final class ForumAttachment: NSObject, NSCoding { + + static let maxFileSize = 2_097_152 + static let maxDimension = 4096 + static let supportedExtensions = ["gif", "jpg", "jpeg", "png"] + + let image: UIImage? + let photoAssetIdentifier: String? + private(set) var validationError: ValidationError? + + enum ValidationError: Error { + case fileTooLarge(actualSize: Int, maxSize: Int) + case dimensionsTooLarge(width: Int, height: Int, maxDimension: Int) + case unsupportedFormat + case imageDataConversionFailed + } + + init(image: UIImage, photoAssetIdentifier: String? = nil) { + self.image = image + self.photoAssetIdentifier = photoAssetIdentifier + super.init() + self.validationError = performValidation() + } + + required init?(coder: NSCoder) { + if let photoAssetIdentifier = coder.decodeObject(of: NSString.self, forKey: CodingKeys.assetIdentifier.rawValue) { + self.photoAssetIdentifier = photoAssetIdentifier as String + + if let asset = PHAsset.fetchAssets(withLocalIdentifiers: [photoAssetIdentifier as String], options: nil).firstObject { + let options = PHImageRequestOptions() + options.isSynchronous = true + options.deliveryMode = .highQualityFormat + var resultImage: UIImage? + PHImageManager.default().requestImage(for: asset, targetSize: PHImageManagerMaximumSize, contentMode: .default, options: options) { image, _ in + resultImage = image + } + self.image = resultImage + } else { + self.image = nil + } + } else if let imageData = coder.decodeObject(of: NSData.self, forKey: CodingKeys.imageData.rawValue) { + self.image = UIImage(data: imageData as Data) + self.photoAssetIdentifier = nil + } else { + return nil + } + + super.init() + self.validationError = performValidation() + } + + func encode(with coder: NSCoder) { + if let photoAssetIdentifier = photoAssetIdentifier { + coder.encode(photoAssetIdentifier as NSString, forKey: CodingKeys.assetIdentifier.rawValue) + } else if let image = image, let imageData = image.pngData() { + coder.encode(imageData as NSData, forKey: CodingKeys.imageData.rawValue) + } + } + + private enum CodingKeys: String { + case assetIdentifier + case imageData + } + + var isValid: Bool { + return validationError == nil + } + + func validate(maxFileSize: Int? = nil, maxDimension: Int? = nil) -> ValidationError? { + guard let image = image else { + return .imageDataConversionFailed + } + + let effectiveMaxDimension = maxDimension ?? Self.maxDimension + let effectiveMaxFileSize = maxFileSize ?? Self.maxFileSize + + let width = Int(image.size.width * image.scale) + let height = Int(image.size.height * image.scale) + + if width > effectiveMaxDimension || height > effectiveMaxDimension { + return .dimensionsTooLarge(width: width, height: height, maxDimension: effectiveMaxDimension) + } + + do { + let (data, _, _) = try imageData() + if data.count > effectiveMaxFileSize { + return .fileTooLarge(actualSize: data.count, maxSize: effectiveMaxFileSize) + } + } catch { + return .imageDataConversionFailed + } + + return nil + } + + private func performValidation() -> ValidationError? { + return validate() + } + + func imageData() throws -> (data: Data, filename: String, mimeType: String) { + guard let image = image else { + throw ValidationError.imageDataConversionFailed + } + + let hasAlpha = image.hasAlpha + let dateFormatter = DateFormatter() + dateFormatter.dateFormat = "yyyy-MM-dd-HHmmss" + let timestamp = dateFormatter.string(from: Date()) + + if hasAlpha, let pngData = image.pngData() { + return (pngData, "photo-\(timestamp).png", "image/png") + } else if let jpegData = image.jpegData(compressionQuality: 0.9) { + return (jpegData, "photo-\(timestamp).jpg", "image/jpeg") + } else { + throw ValidationError.imageDataConversionFailed + } + } + + func resized(maxDimension: Int? = nil, maxFileSize: Int? = nil) -> ForumAttachment? { + guard let originalImage = image else { return nil } + + let effectiveMaxDimension = maxDimension ?? Self.maxDimension + let effectiveMaxFileSize = maxFileSize ?? Self.maxFileSize + + let originalWidth = Int(originalImage.size.width * originalImage.scale) + let originalHeight = Int(originalImage.size.height * originalImage.scale) + + var targetWidth = originalWidth + var targetHeight = originalHeight + + if originalWidth > effectiveMaxDimension || originalHeight > effectiveMaxDimension { + let ratio = min(CGFloat(effectiveMaxDimension) / CGFloat(originalWidth), + CGFloat(effectiveMaxDimension) / CGFloat(originalHeight)) + targetWidth = Int(CGFloat(originalWidth) * ratio) + targetHeight = Int(CGFloat(originalHeight) * ratio) + } + + var compressionQuality: CGFloat = 0.9 + var resizedImage = originalImage.resized(to: CGSize(width: targetWidth, height: targetHeight)) + + while compressionQuality > 0.1 { + let hasAlpha = resizedImage.hasAlpha + let data: Data? + + if hasAlpha { + data = resizedImage.pngData() + } else { + data = resizedImage.jpegData(compressionQuality: compressionQuality) + } + + if let imageData = data, imageData.count <= effectiveMaxFileSize { + return ForumAttachment(image: resizedImage, photoAssetIdentifier: photoAssetIdentifier) + } + + if hasAlpha { + let newWidth = Int(CGFloat(targetWidth) * 0.9) + let newHeight = Int(CGFloat(targetHeight) * 0.9) + if newWidth < 100 || newHeight < 100 { break } + targetWidth = newWidth + targetHeight = newHeight + resizedImage = originalImage.resized(to: CGSize(width: targetWidth, height: targetHeight)) + } else { + compressionQuality -= 0.1 + } + } + + return nil + } +} + +private extension UIImage { + var hasAlpha: Bool { + guard let alphaInfo = cgImage?.alphaInfo else { return false } + return alphaInfo == .first || alphaInfo == .last || alphaInfo == .premultipliedFirst || alphaInfo == .premultipliedLast + } + + func resized(to targetSize: CGSize) -> UIImage { + let format = UIGraphicsImageRendererFormat() + format.scale = 1 + let renderer = UIGraphicsImageRenderer(size: targetSize, format: format) + return renderer.image { _ in + draw(in: CGRect(origin: .zero, size: targetSize)) + } + } +} diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index 50f66aa98..2cff145db 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -2,7 +2,9 @@ // // Copyright 2017 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app +import AwfulCore import HTMLReader +import UIKit extension HTMLDocument { @@ -145,9 +147,42 @@ extension HTMLDocument { let src = img["src"], let url = URL(string: src) else { continue } - + + // Handle attachment images by adding a placeholder and data attribute + // These will be loaded asynchronously via JavaScript after page load + if src.contains("attachment.php") { + let attachmentSrc: String + if src.hasPrefix("http") { + attachmentSrc = src + } else if let baseURL = ForumsClient.shared.baseURL { + attachmentSrc = baseURL.absoluteString + src + } else { + continue + } + + // Extract postid from URL + if let urlComponents = URLComponents(string: attachmentSrc), + let postIDItem = urlComponents.queryItems?.first(where: { $0.name == "postid" }), + let postID = postIDItem.value { + + // Mark as attachment image and store the postID for async loading + img["data-awful-attachment-postid"] = postID + img["data-awful-attachment-url"] = attachmentSrc + // Use a placeholder while loading + img["src"] = "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='100' height='100'%3E%3Crect fill='%23ddd' width='100' height='100'/%3E%3Ctext x='50%25' y='50%25' dominant-baseline='middle' text-anchor='middle' fill='%23999' font-family='sans-serif' font-size='14'%3ELoading...%3C/text%3E%3C/svg%3E" + } + continue + } + + // Fix relative non-attachment URLs to be absolute + if !src.hasPrefix("http") { + if let baseURL = ForumsClient.shared.baseURL { + img["src"] = baseURL.absoluteString + src + } + } + let isSmilie = isSmilieURL(url) - + if isSmilie { img.toggleClass("awful-smile") } else if let postimageURL = fixPostimageURL(url) { @@ -155,11 +190,12 @@ extension HTMLDocument { } else if let waffleURL = randomwaffleURLForWaffleimagesURL(url) { img["src"] = waffleURL.absoluteString } - + if shouldLinkifyNonSmilies, !isSmilie { let link = HTMLElement(tagName: "span", attributes: [ "data-awful-linkified-image": ""]) - link.textContent = src + // Use the updated src attribute after any URL fixes + link.textContent = img["src"] ?? src img.parent?.replace(child: img, with: link) } } diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index c25a17882..e1f13d5ed 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -94,10 +94,13 @@ final class ReplyWorkspace: NSObject { Unfortunately, any compositionViewController that we preserve in encodeRestorableStateWithCoder() is not yet available in objectWithRestorationIdentifierPath(_:coder:); it only becomes available in decodeRestorableStateWithCoder(). This didSet encompasses the junk we want to set up on the compositionViewController no matter how it's created and really belongs in init(), except we're stuck. */ - fileprivate var compositionViewController: CompositionViewController! { + var compositionViewController: CompositionViewController! { didSet { assert(oldValue == nil, "please set compositionViewController only once") - + + // Ensure the view is loaded before accessing textView + compositionViewController.loadViewIfNeeded() + let textView = compositionViewController.textView textView.attributedText = draft.text @@ -116,11 +119,11 @@ final class ReplyWorkspace: NSObject { textViewNotificationToken = NotificationCenter.default.addObserver(forName: UITextView.textDidChangeNotification, object: compositionViewController.textView, queue: OperationQueue.main) { [unowned self] note in self.rightButtonItem.isEnabled = textView.hasText } - + let navigationItem = compositionViewController.navigationItem navigationItem.leftBarButtonItem = UIBarButtonItem(barButtonSystemItem: .cancel, target: self, action: #selector(ReplyWorkspace.didTapCancel(_:))) navigationItem.rightBarButtonItem = rightButtonItem - + $confirmBeforeReplying .receive(on: RunLoop.main) .sink { [weak self] _ in self?.updateRightButtonItem() } @@ -133,6 +136,8 @@ final class ReplyWorkspace: NSObject { textView.autocorrectionType = tweaks.autocorrectionType textView.spellCheckingType = tweaks.spellCheckingType } + + compositionViewController.setDraft(draft) } } @@ -348,6 +353,8 @@ extension ReplyWorkspace: UIObjectRestoration, UIStateRestoring { var thread: AwfulThread { get } var text: NSAttributedString? { get set } var title: String { get } + var forumAttachment: ForumAttachment? { get set } + var shouldDeleteAttachment: Bool { get set } } @objc protocol SubmittableDraft { @@ -362,28 +369,36 @@ extension ReplyWorkspace: UIObjectRestoration, UIStateRestoring { final class NewReplyDraft: NSObject, ReplyDraft { let thread: AwfulThread var text: NSAttributedString? - + var forumAttachment: ForumAttachment? + var existingAttachmentInfo: (id: String, filename: String)? + var shouldDeleteAttachment = false + init(thread: AwfulThread, text: NSAttributedString? = nil) { self.thread = thread self.text = text super.init() } - + convenience init?(coder: NSCoder) { let threadKey = coder.decodeObject(forKey: Keys.threadKey) as! ThreadKey let thread = AwfulThread.objectForKey(objectKey: threadKey, in: AppDelegate.instance.managedObjectContext) let text = coder.decodeObject(forKey: Keys.text) as? NSAttributedString self.init(thread: thread, text: text) + self.forumAttachment = coder.decodeObject(of: ForumAttachment.self, forKey: Keys.forumAttachment) } - + func encode(with coder: NSCoder) { coder.encode(thread.objectKey, forKey: Keys.threadKey) coder.encode(text, forKey: Keys.text) + if let forumAttachment = forumAttachment { + coder.encode(forumAttachment, forKey: Keys.forumAttachment) + } } - + fileprivate struct Keys { static let threadKey = "threadKey" static let text = "text" + static let forumAttachment = "forumAttachment" } var storePath: String { @@ -396,30 +411,66 @@ final class NewReplyDraft: NSObject, ReplyDraft { } final class EditReplyDraft: NSObject, ReplyDraft { + enum AttachmentAction { + case keep + case delete + } + let post: Post var text: NSAttributedString? - + var forumAttachment: ForumAttachment? + var existingAttachmentInfo: (id: String, filename: String)? + var shouldDeleteAttachment = false + + var attachmentAction: AttachmentAction { + get { shouldDeleteAttachment ? .delete : .keep } + set { shouldDeleteAttachment = (newValue == .delete) } + } + + var existingAttachmentFilename: String? { + return existingAttachmentInfo?.filename + } + + var existingAttachmentFilesize: String? { + // We don't have filesize info from the server, so return nil + return nil + } + init(post: Post, text: NSAttributedString? = nil) { self.post = post self.text = text super.init() } - + convenience init?(coder: NSCoder) { let postKey = coder.decodeObject(forKey: Keys.postKey) as! PostKey let post = Post.objectForKey(objectKey: postKey, in: AppDelegate.instance.managedObjectContext) let text = coder.decodeObject(forKey: Keys.text) as? NSAttributedString self.init(post: post, text: text) + + if let attachmentID = coder.decodeObject(of: NSString.self, forKey: Keys.attachmentID) as? String, + let attachmentFilename = coder.decodeObject(of: NSString.self, forKey: Keys.attachmentFilename) as? String { + self.existingAttachmentInfo = (id: attachmentID, filename: attachmentFilename) + } + self.shouldDeleteAttachment = coder.decodeBool(forKey: Keys.shouldDeleteAttachment) } - + func encode(with coder: NSCoder) { coder.encode(post.objectKey, forKey: Keys.postKey) coder.encode(text, forKey: Keys.text) + if let existingAttachmentInfo = existingAttachmentInfo { + coder.encode(existingAttachmentInfo.id as NSString, forKey: Keys.attachmentID) + coder.encode(existingAttachmentInfo.filename as NSString, forKey: Keys.attachmentFilename) + } + coder.encode(shouldDeleteAttachment, forKey: Keys.shouldDeleteAttachment) } - + fileprivate struct Keys { static let postKey = "postKey" static let text = "text" + static let attachmentID = "attachmentID" + static let attachmentFilename = "attachmentFilename" + static let shouldDeleteAttachment = "shouldDeleteAttachment" } var thread: AwfulThread { @@ -444,7 +495,24 @@ extension NewReplyDraft: SubmittableDraft { } else { Task { @MainActor in do { - _ = try await ForumsClient.shared.reply(to: thread, bbcode: plainText ?? "") + var attachmentData: (data: Data, filename: String, mimeType: String)? + if let forumAttachment = forumAttachment { + let limits = try await ForumsClient.shared.fetchAttachmentLimits(for: thread) + + if let validationError = forumAttachment.validate( + maxFileSize: limits.maxFileSize, + maxDimension: limits.maxDimension + ) { + let error = NSError(domain: "Awful", code: 0, userInfo: [ + NSLocalizedDescriptionKey: self.validationErrorMessage(for: validationError) + ]) + completion(error) + return + } + attachmentData = try forumAttachment.imageData() + } + + _ = try await ForumsClient.shared.reply(to: thread, bbcode: plainText ?? "", attachment: attachmentData) completion(nil) } catch { completion(error) @@ -453,6 +521,21 @@ extension NewReplyDraft: SubmittableDraft { } } } + + private func validationErrorMessage(for error: ForumAttachment.ValidationError) -> String { + switch error { + case .fileTooLarge(let actualSize, let maxSize): + let actualMB = Double(actualSize) / 1_048_576 + let maxMB = Double(maxSize) / 1_048_576 + return String(format: "Image is too large (%.1f MB). Maximum size is %.1f MB.", actualMB, maxMB) + case .dimensionsTooLarge(let width, let height, let maxDimension): + return "Image dimensions (\(width)×\(height)) exceed maximum (\(maxDimension)×\(maxDimension))." + case .unsupportedFormat: + return "Unsupported image format. Supported formats: GIF, JPEG, PNG" + case .imageDataConversionFailed: + return "Failed to convert image data" + } + } } extension EditReplyDraft: SubmittableDraft { @@ -463,7 +546,8 @@ extension EditReplyDraft: SubmittableDraft { } else { Task { @MainActor in do { - try await ForumsClient.shared.edit(post, bbcode: plainText ?? "") + let attachmentAction: ForumsClient.AttachmentAction = shouldDeleteAttachment ? .delete : .keep + try await ForumsClient.shared.edit(post, bbcode: plainText ?? "", attachmentAction: attachmentAction) completion(nil) } catch { completion(error) diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 24f7ffac2..1b6b19bb4 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -953,6 +953,30 @@ Awful.embedGfycat = function() { } Awful.embedGfycat(); + +// Load attachment images asynchronously +Awful.loadAttachmentImages = function() { + var attachmentImages = document.querySelectorAll('img[data-awful-attachment-postid]'); + attachmentImages.forEach(function(img) { + var postID = img.getAttribute('data-awful-attachment-postid'); + var id = 'attachment-' + Date.now() + '-' + Math.random(); + img.setAttribute('data-awful-attachment-id', id); + window.webkit.messageHandlers.fetchAttachmentImage.postMessage({ + id: id, + postid: postID + }); + }); +}; + +Awful.didFetchAttachmentImage = function(id, dataURL) { + var img = document.querySelector('img[data-awful-attachment-id="' + id + '"]'); + if (img) { + img.src = dataURL; + } +}; + +Awful.loadAttachmentImages(); + // THIS SHOULD STAY AT THE BOTTOM OF THE FILE! // All done; tell the native side we're ready. window.webkit.messageHandlers.didRender.postMessage({}); diff --git a/App/View Controllers/Posts/PostsPageViewController.swift b/App/View Controllers/Posts/PostsPageViewController.swift index 31e12e867..0ac78962b 100644 --- a/App/View Controllers/Posts/PostsPageViewController.swift +++ b/App/View Controllers/Posts/PostsPageViewController.swift @@ -105,6 +105,7 @@ final class PostsPageViewController: ViewController { postsView.renderView.registerMessage(RenderView.BuiltInMessage.DidTapPostActionButton.self) postsView.renderView.registerMessage(RenderView.BuiltInMessage.DidTapAuthorHeader.self) postsView.renderView.registerMessage(RenderView.BuiltInMessage.FetchOEmbedFragment.self) + postsView.renderView.registerMessage(RenderView.BuiltInMessage.FetchAttachmentImage.self) postsView.topBar.goToParentForum = { [unowned self] in guard let forum = self.thread.forum else { return } AppDelegate.instance.open(route: .forum(id: forum.forumID)) @@ -1249,7 +1250,15 @@ final class PostsPageViewController: ViewController { Task { do { let text = try await ForumsClient.shared.findBBcodeContents(of: selectedPost!) + let attachmentInfo = try? await ForumsClient.shared.findAttachmentInfo(for: selectedPost!) let replyWorkspace = ReplyWorkspace(post: selectedPost!, bbcode: text) + + // Set attachment info before creating the composition view controller + if let attachmentInfo = attachmentInfo, + let editDraft = replyWorkspace.draft as? EditReplyDraft { + editDraft.existingAttachmentInfo = attachmentInfo + } + self.replyWorkspace = replyWorkspace replyWorkspace.completion = replyCompletionBlock present(replyWorkspace.viewController, animated: true) @@ -1365,6 +1374,23 @@ final class PostsPageViewController: ViewController { } } + private func fetchAttachmentImage(postID: String, id: String) { + Task { + do { + let imageData = try await ForumsClient.shared.fetchAttachmentImage(postID: postID) + if let image = UIImage(data: imageData), let pngData = image.pngData() { + let base64 = pngData.base64EncodedString() + let dataURL = "data:image/png;base64,\(base64)" + await MainActor.run { + postsView.renderView.didFetchAttachmentImage(id: id, dataURL: dataURL) + } + } + } catch { + logger.error("Failed to fetch attachment image for postID \(postID): \(error)") + } + } + } + private func presentDraftMenu( from source: DraftMenuSource, options: DraftMenuOptions @@ -1792,6 +1818,9 @@ extension PostsPageViewController: RenderViewDelegate { case let message as RenderView.BuiltInMessage.FetchOEmbedFragment: fetchOEmbed(url: message.url, id: message.id) + case let message as RenderView.BuiltInMessage.FetchAttachmentImage: + fetchAttachmentImage(postID: message.postID, id: message.id) + case is FYADFlagRequest: fetchNewFlag() diff --git a/App/Views/RenderView.swift b/App/Views/RenderView.swift index 2bc995bea..45db9aa16 100644 --- a/App/Views/RenderView.swift +++ b/App/Views/RenderView.swift @@ -260,13 +260,13 @@ extension RenderView: WKScriptMessageHandler { struct FetchOEmbedFragment: RenderViewMessage { static let messageName = "fetchOEmbedFragment" - + /// An opaque `id` to use when calling back with the response. let id: String - + /// The OEmbed URL to fetch. let url: URL - + init?(rawMessage: WKScriptMessage, in renderView: RenderView) { assert(rawMessage.name == Self.messageName) guard let body = rawMessage.body as? [String: Any], @@ -274,11 +274,32 @@ extension RenderView: WKScriptMessageHandler { let rawURL = body["url"] as? String, let url = URL(string: rawURL) else { return nil } - + self.id = id self.url = url } } + + struct FetchAttachmentImage: RenderViewMessage { + static let messageName = "fetchAttachmentImage" + + /// An opaque `id` to use when calling back with the response. + let id: String + + /// The post ID for the attachment. + let postID: String + + init?(rawMessage: WKScriptMessage, in renderView: RenderView) { + assert(rawMessage.name == Self.messageName) + guard let body = rawMessage.body as? [String: Any], + let id = body["id"] as? String, + let postID = body["postid"] as? String + else { return nil } + + self.id = id + self.postID = postID + } + } } } @@ -505,6 +526,18 @@ extension RenderView { } } + func didFetchAttachmentImage(id: String, dataURL: String) { + Task { + do { + try await webView.eval(""" + window.Awful?.didFetchAttachmentImage(\(escapeForEval(id)), \(escapeForEval(dataURL))); + """) + } catch { + logger.error("error calling back after fetching attachment image: \(error)") + } + } + } + /** How far the web document is offset from the scroll view's bounds. diff --git a/Awful.xcodeproj/project.pbxproj b/Awful.xcodeproj/project.pbxproj index b32003490..ae3c057b9 100644 --- a/Awful.xcodeproj/project.pbxproj +++ b/Awful.xcodeproj/project.pbxproj @@ -199,11 +199,14 @@ 2D19BA3929C33303009DD94F /* niggly60.json in Resources */ = {isa = PBXBuildFile; fileRef = 2D19BA3629C33302009DD94F /* niggly60.json */; }; 2D19BA3A29C33303009DD94F /* frogrefresh60.json in Resources */ = {isa = PBXBuildFile; fileRef = 2D19BA3729C33302009DD94F /* frogrefresh60.json */; }; 2D19BA3B29C33303009DD94F /* toot60.json in Resources */ = {isa = PBXBuildFile; fileRef = 2D19BA3829C33302009DD94F /* toot60.json */; }; + 2D21C8822EC1C6730089284B /* ForumAttachment.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D21C8812EC1C6730089284B /* ForumAttachment.swift */; }; 2D265F8C292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D265F8B292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift */; }; 2D265F8F292CB447001336ED /* Lottie in Frameworks */ = {isa = PBXBuildFile; productRef = 2D265F8E292CB447001336ED /* Lottie */; }; 2D327DD627F468CE00D21AB0 /* BookmarkColorPicker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D327DD527F468CE00D21AB0 /* BookmarkColorPicker.swift */; }; 2D921269292F588100B16011 /* platinum-member.png in Resources */ = {isa = PBXBuildFile; fileRef = 2D921268292F588100B16011 /* platinum-member.png */; }; 2DAF1FE12E05D3ED006F6BC4 /* View+FontDesign.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DAF1FE02E05D3EB006F6BC4 /* View+FontDesign.swift */; }; + 2DB447302EC1E2DA00F03402 /* AttachmentPreviewView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DB4472F2EC1E2DA00F03402 /* AttachmentPreviewView.swift */; }; + 2DB447312EC1E2DA00F03402 /* AttachmentEditView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DB4472E2EC1E2DA00F03402 /* AttachmentEditView.swift */; }; 2DD8209C25DDD9BF0015A90D /* CopyImageActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DD8209B25DDD9BF0015A90D /* CopyImageActivity.swift */; }; 306F740B2D90AA01000717BC /* KeychainAccess in Frameworks */ = {isa = PBXBuildFile; productRef = 306F740A2D90AA01000717BC /* KeychainAccess */; }; 30E0C51D2E35C89D0030DC0A /* AnimatedImageView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30E0C5162E35C89D0030DC0A /* AnimatedImageView.swift */; }; @@ -515,13 +518,15 @@ 2D19BA3629C33302009DD94F /* niggly60.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = niggly60.json; sourceTree = ""; }; 2D19BA3729C33302009DD94F /* frogrefresh60.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = frogrefresh60.json; sourceTree = ""; }; 2D19BA3829C33302009DD94F /* toot60.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = toot60.json; sourceTree = ""; }; + 2D21C8812EC1C6730089284B /* ForumAttachment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ForumAttachment.swift; sourceTree = ""; }; 2D265F8B292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GetOutFrogRefreshSpinnerView.swift; sourceTree = ""; }; 2D327DD527F468CE00D21AB0 /* BookmarkColorPicker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkColorPicker.swift; sourceTree = ""; }; 2D921268292F588100B16011 /* platinum-member.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "platinum-member.png"; sourceTree = ""; }; 2DAF1FE02E05D3EB006F6BC4 /* View+FontDesign.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "View+FontDesign.swift"; sourceTree = ""; }; + 2DB4472E2EC1E2DA00F03402 /* AttachmentEditView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttachmentEditView.swift; sourceTree = ""; }; + 2DB4472F2EC1E2DA00F03402 /* AttachmentPreviewView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttachmentPreviewView.swift; sourceTree = ""; }; 2DD8209B25DDD9BF0015A90D /* CopyImageActivity.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CopyImageActivity.swift; sourceTree = ""; }; 30E0C5162E35C89D0030DC0A /* AnimatedImageView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnimatedImageView.swift; sourceTree = ""; }; - 30E0C5172E35C89D0030DC0A /* SmilieData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SmilieData.swift; sourceTree = ""; }; 30E0C5182E35C89D0030DC0A /* SmilieGridItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SmilieGridItem.swift; sourceTree = ""; }; 30E0C5192E35C89D0030DC0A /* SmiliePickerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SmiliePickerView.swift; sourceTree = ""; }; 30E0C51A2E35C89D0030DC0A /* SmilieSearchViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SmilieSearchViewModel.swift; sourceTree = ""; }; @@ -1098,6 +1103,9 @@ 1CF2462016DD957300C75E05 /* Composition */ = { isa = PBXGroup; children = ( + 2DB4472E2EC1E2DA00F03402 /* AttachmentEditView.swift */, + 2DB4472F2EC1E2DA00F03402 /* AttachmentPreviewView.swift */, + 2D21C8812EC1C6730089284B /* ForumAttachment.swift */, 1C2C1F0D1CE16FE200CD27DD /* CloseBBcodeTagCommand.swift */, 1C16FB9F1CB492C600C88BD1 /* ComposeField.swift */, 1C16FBA31CB4A41C00C88BD1 /* ComposeTextView.swift */, @@ -1541,7 +1549,6 @@ 83410EF219A582B8002CD019 /* DateFormatters.swift in Sources */, 1C273A9E21B316DB002875A9 /* LoadMoreFooter.swift in Sources */, 1C2C1F0E1CE16FE200CD27DD /* CloseBBcodeTagCommand.swift in Sources */, - 30E0C51C2E35C89D0030DC0A /* SmilieData.swift in Sources */, 30E0C51D2E35C89D0030DC0A /* AnimatedImageView.swift in Sources */, 30E0C51E2E35C89D0030DC0A /* SmiliePickerView.swift in Sources */, 30E0C51F2E35C89D0030DC0A /* SmilieSearchViewModel.swift in Sources */, @@ -1580,6 +1587,7 @@ 1C16FBBA1CB8961F00C88BD1 /* ImageURLProtocol.swift in Sources */, 1C09BFF21A09F86A007C11F5 /* InAppActionCollectionView.swift in Sources */, 1C48538E1F93BADF0042531A /* HTMLRenderingHelpers.swift in Sources */, + 2D21C8822EC1C6730089284B /* ForumAttachment.swift in Sources */, 1C16FC081CC3F01B00C88BD1 /* RapSheetViewController.swift in Sources */, 1CE55A7A1A1072D900E474A6 /* ForumsTableViewController.swift in Sources */, 1C42A2271FD47AEB00F67BA1 /* Forum+Presentation.swift in Sources */, @@ -1661,6 +1669,8 @@ 1C16FBD71CBAA00200C88BD1 /* PostsPageTopBar.swift in Sources */, 1CC256B31A3876F7003FA7A8 /* CompositionViewController.swift in Sources */, 1C16FBFC1CBF0F6B00C88BD1 /* ThreadTagPickerCell.swift in Sources */, + 2DB447302EC1E2DA00F03402 /* AttachmentPreviewView.swift in Sources */, + 2DB447312EC1E2DA00F03402 /* AttachmentEditView.swift in Sources */, 1CC256BF1A3AC9C0003FA7A8 /* CompositionMenuTree.swift in Sources */, 1CC6645D220D224C00BEF5A6 /* Environment.swift in Sources */, 1C8F6807222B6DD9007E61ED /* ThreadTagDataLoader.swift in Sources */, diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index 11f2774c9..80895cc53 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -792,8 +792,9 @@ public final class ForumsClient { public func reply( to thread: AwfulThread, - bbcode: String - ) async throws -> ReplyLocation { + bbcode: String, + attachment: (data: Data, filename: String, mimeType: String)? = nil + ) async throws -> (location: ReplyLocation, attachmentLimits: (maxFileSize: Int, maxDimension: Int)?) { guard let mainContext = managedObjectContext else { throw Error.missingManagedObjectContext } @@ -802,12 +803,16 @@ public final class ForumsClient { (thread.closed, thread.threadID) } let formParams: [KeyValuePairs.Element] + var parsedLimits: (maxFileSize: Int, maxDimension: Int)? do { let (data, response) = try await fetch(method: .get, urlString: "newreply.php", parameters: [ "action": "newreply", "threadid": threadID, ]) let (document, url) = try parseHTML(data: data, response: response) + + parsedLimits = parseAttachmentLimits(from: document) + guard let htmlForm = document.firstNode(matchingSelector: "form[name='vbform']") else { let description = if wasThreadClosed { "Could not reply; the thread may be closed." @@ -822,10 +827,26 @@ public final class ForumsClient { let submission = form.submit(button: parsedForm.submitButton(named: "submit")) formParams = prepareFormEntries(submission) } - + let postID: String? do { - let (data, response) = try await fetch(method: .post, urlString: "newreply.php", parameters: formParams) + let (data, response): (Data, URLResponse) + if let attachment = attachment { + guard let urlSession else { + throw Error.missingURLSession + } + guard let url = URL(string: "newreply.php", relativeTo: baseURL) else { + throw Error.invalidBaseURL + } + var request = URLRequest(url: url) + request.httpMethod = "POST" + let escapedParams = formParams.lazy.map(win1252Escaped(_:)) + try request.setMultipartFormData(escapedParams, encoding: .windowsCP1252) + try request.appendFileData(attachment.data, withName: "attachment", filename: attachment.filename, mimeType: attachment.mimeType) + (data, response) = try await urlSession.data(for: request) + } else { + (data, response) = try await fetch(method: .post, urlString: "newreply.php", parameters: formParams) + } let (document, _) = try parseHTML(data: data, response: response) let link = document.firstNode(matchingSelector: "a[href *= 'goto=post']") ?? document.firstNode(matchingSelector: "a[href *= 'goto=lastpost']") @@ -839,13 +860,93 @@ public final class ForumsClient { nil } } - return await mainContext.perform { + let location = await mainContext.perform { if let postID { - .post(Post.objectForKey(objectKey: PostKey(postID: postID), in: mainContext)) + ReplyLocation.post(Post.objectForKey(objectKey: PostKey(postID: postID), in: mainContext)) } else { - .lastPostInThread + ReplyLocation.lastPostInThread + } + } + return (location: location, attachmentLimits: parsedLimits) + } + + public func fetchAttachmentLimits(for thread: AwfulThread) async throws -> (maxFileSize: Int, maxDimension: Int) { + let threadID: String = await thread.managedObjectContext!.perform { + thread.threadID + } + let (data, response) = try await fetch(method: .get, urlString: "newreply.php", parameters: [ + "action": "newreply", + "threadid": threadID, + ]) + let (document, _) = try parseHTML(data: data, response: response) + + if let limits = parseAttachmentLimits(from: document) { + return limits + } + + return (maxFileSize: 2_097_152, maxDimension: 4096) + } + + private func parseAttachmentLimits(from document: HTMLDocument) -> (maxFileSize: Int, maxDimension: Int)? { + guard let maxFileSizeString = document.firstNode(matchingSelector: "input[name='MAX_FILE_SIZE']")?["value"], + let maxFileSize = Int(maxFileSizeString) else { + return nil + } + + var maxDimension = 4096 + for td in document.nodes(matchingSelector: "td") { + let text = td.textContent + if text.contains("Attach file:") { + var nextSibling: HTMLElement? + if let sibling = td.nextSibling as? HTMLElement { + nextSibling = sibling + } else if let parent = td.parent { + let children = parent.children.compactMap { $0 as? HTMLElement } + if let index = children.firstIndex(where: { $0 === td }), index + 1 < children.count { + nextSibling = children[index + 1] + } + } + + if let nextSibling = nextSibling { + let limitsText = nextSibling.textContent + let pattern = #"dimensions:\s*(\d+)x(\d+)"# + if let regex = try? NSRegularExpression(pattern: pattern, options: [.caseInsensitive]), + let match = regex.firstMatch(in: limitsText, range: NSRange(limitsText.startIndex..., in: limitsText)), + let dimensionRange = Range(match.range(at: 1), in: limitsText), + let parsedDimension = Int(limitsText[dimensionRange]) { + maxDimension = parsedDimension + } + } + break } } + + return (maxFileSize: maxFileSize, maxDimension: maxDimension) + } + + public func fetchAttachmentImage(postID: String) async throws -> Data { + guard let url = URL(string: "attachment.php?postid=\(postID)", relativeTo: baseURL) else { + throw Error.invalidBaseURL + } + + var request = URLRequest(url: url) + request.httpMethod = "GET" + + guard let session = urlSession else { + throw Error.invalidBaseURL + } + + let (data, response) = try await session.data(for: request) + + guard let httpResponse = response as? HTTPURLResponse else { + throw URLError(.badServerResponse) + } + + guard httpResponse.statusCode == 200 else { + throw URLError(.badServerResponse) + } + + return data } public func previewReply( @@ -909,14 +1010,26 @@ public final class ForumsClient { return try findMessageText(in: parsed) } + public enum AttachmentAction { + case keep + case delete + } + public func edit( _ post: Post, - bbcode: String + bbcode: String, + attachmentAction: AttachmentAction = .keep ) async throws { let params: [KeyValuePairs.Element] do { let parsedForm = try await editForm(for: post) let form = SubmittableForm(parsedForm) + + if parsedForm.controls.contains(where: { $0.name == "attachmentaction" }) { + let value = attachmentAction == .keep ? "keep" : "delete" + try form.select(value: value, for: "attachmentaction") + } + try form.enter(text: bbcode, for: "message") let submission = form.submit(button: parsedForm.submitButton(named: "submit")) params = prepareFormEntries(submission) @@ -947,6 +1060,29 @@ public final class ForumsClient { return try Form(htmlForm, url: url) } + public func findAttachmentInfo(for post: Post) async throws -> (id: String, filename: String)? { + let (data, response) = try await fetch(method: .get, urlString: "editpost.php", parameters: [ + "action": "editpost", + "postid": await post.managedObjectContext!.perform { post.postID }, + ]) + let (document, _) = try parseHTML(data: data, response: response) + + guard let attachmentLink = document.firstNode(matchingSelector: "a[href*='attachment.php']"), + let href = attachmentLink["href"] else { + return nil + } + + let filename = attachmentLink.textContent.trimmingCharacters(in: .whitespacesAndNewlines) + guard !filename.isEmpty, + let components = URLComponents(string: href), + let queryItems = components.queryItems, + let attachmentID = queryItems.first(where: { $0.name == "attachmentid" })?.value else { + return nil + } + + return (id: attachmentID, filename: filename) + } + /** - Parameter postID: The post's ID. Specified directly in case no such post exists, which would make for a useless `Post`. - Returns: The promise of a post (with its `thread` set) and the page containing the post (may be `AwfulThreadPage.last`). diff --git a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift index fdc1114bc..c3f7d2823 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift @@ -59,6 +59,44 @@ extension URLRequest { }() } + mutating func appendFileData( + _ data: Data, + withName name: String, + filename: String, + mimeType: String + ) throws { + guard var existingBody = httpBody else { + throw EncodingError("httpBody not set; call setMultipartFormData first") + } + + guard let contentType = value(forHTTPHeaderField: "Content-Type"), + contentType.hasPrefix("multipart/form-data"), + let boundaryRange = contentType.range(of: "boundary="), + boundaryRange.upperBound < contentType.endIndex else { + throw EncodingError("Content-Type not set or invalid; call setMultipartFormData first") + } + + let boundary = String(contentType[boundaryRange.upperBound...]).trimmingCharacters(in: .whitespaces) + + let lastBoundary = "--\(boundary)--\r\n".data(using: .utf8)! + if existingBody.suffix(lastBoundary.count) == lastBoundary { + existingBody.removeLast(lastBoundary.count) + } else { + throw EncodingError("httpBody does not end with expected boundary") + } + + var filePartData = Data() + filePartData.append("\r\n--\(boundary)\r\n".data(using: .utf8)!) + filePartData.append("Content-Disposition: form-data; name=\"\(name)\"; filename=\"\(filename)\"\r\n".data(using: .utf8)!) + filePartData.append("Content-Type: \(mimeType)\r\n".data(using: .utf8)!) + filePartData.append("\r\n".data(using: .utf8)!) + filePartData.append(data) + filePartData.append("\r\n--\(boundary)--\r\n".data(using: .utf8)!) + + existingBody.append(filePartData) + httpBody = existingBody + } + struct EncodingError: Error { let what: String init(_ what: String) { From 871058cdb2db66edaed37c6ce85f4dae99c6ef39 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 14:49:06 +1100 Subject: [PATCH 02/13] Refactored to improve code quality --- App/Composition/AttachmentEditView.swift | 19 ++++++++---- App/Composition/CompositionMenuTree.swift | 23 ++------------ .../CompositionViewController.swift | 6 ++-- App/Composition/ForumAttachment.swift | 17 ++++++++++ App/Posts/ReplyWorkspace.swift | 31 ++++++++----------- .../Posts/PostsPageViewController.swift | 13 +++++++- .../AwfulCore/Networking/ForumsClient.swift | 2 ++ 7 files changed, 63 insertions(+), 48 deletions(-) diff --git a/App/Composition/AttachmentEditView.swift b/App/Composition/AttachmentEditView.swift index 3b3036d7c..406fdc747 100644 --- a/App/Composition/AttachmentEditView.swift +++ b/App/Composition/AttachmentEditView.swift @@ -114,7 +114,7 @@ final class AttachmentEditView: UIView { onActionChanged?(action) } - func configure(filename: String, filesize: String?) { + func configure(filename: String, filesize: String?, image: UIImage? = nil) { titleLabel.text = "Current Attachment" if let filesize = filesize { detailLabel.text = "\(filename) • \(filesize)" @@ -122,11 +122,18 @@ final class AttachmentEditView: UIView { detailLabel.text = filename } - // Show a generic document icon since we don't have the actual image - let config = UIImage.SymbolConfiguration(pointSize: 40, weight: .light) - imageView.image = UIImage(systemName: "doc.fill", withConfiguration: config) - imageView.tintColor = .tertiaryLabel - imageView.contentMode = .center + if let image = image { + // Display the actual attachment image + imageView.image = image + imageView.tintColor = nil + imageView.contentMode = .scaleAspectFit + } else { + // Show a generic document icon as fallback + let config = UIImage.SymbolConfiguration(pointSize: 40, weight: .light) + imageView.image = UIImage(systemName: "doc.fill", withConfiguration: config) + imageView.tintColor = .tertiaryLabel + imageView.contentMode = .center + } } var selectedAction: AttachmentAction { diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 22bb21c8b..92176d0c7 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -292,21 +292,6 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont textView.becomeFirstResponder() } - private func validationErrorMessage(for error: ForumAttachment.ValidationError) -> String { - switch error { - case .fileTooLarge(let actualSize, let maxSize): - let formatter = ByteCountFormatter() - formatter.countStyle = .file - return "File size (\(formatter.string(fromByteCount: Int64(actualSize)))) exceeds maximum (\(formatter.string(fromByteCount: Int64(maxSize))))" - case .dimensionsTooLarge(let width, let height, let maxDimension): - return "Image dimensions (\(width)x\(height)) exceed maximum (\(maxDimension)x\(maxDimension))" - case .unsupportedFormat: - return "Unsupported image format. Please use GIF, JPG, JPEG, or PNG" - case .imageDataConversionFailed: - return "Failed to process image data" - } - } - fileprivate func useImageHostForPendingImage() { guard let image = pendingImage else { return } @@ -333,7 +318,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont if canResize(error) { let alert = UIAlertController( title: "Attachment Too Large", - message: "\(validationErrorMessage(for: error))\n\nWould you like to automatically resize the image to fit?", + message: "\(error.localizedDescription)\n\nWould you like to automatically resize the image to fit?", preferredStyle: .alert ) alert.addAction(UIAlertAction(title: "Cancel", style: .cancel) { [weak self] _ in @@ -347,7 +332,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } else { let alert = UIAlertController( title: "Invalid Attachment", - message: validationErrorMessage(for: error), + message: error.localizedDescription, alertActions: [.ok()] ) textView.nearestViewController?.present(alert, animated: true) @@ -362,7 +347,6 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont pendingImageAssetIdentifier = nil onAttachmentChanged?() - // Success message removed - the preview card is sufficient feedback } private func canResize(_ error: ForumAttachment.ValidationError) -> Bool { @@ -412,7 +396,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont let alert = UIAlertController( title: "Resize Failed", - message: "\(self.validationErrorMessage(for: error))\n\nPlease try a different image.", + message: "\(error.localizedDescription)\n\nPlease try a different image.", alertActions: [.ok()] ) self.textView.nearestViewController?.present(alert, animated: true) @@ -422,7 +406,6 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont // Update with resized attachment self.draft?.forumAttachment = resizedAttachment self.onAttachmentChanged?() - // Success message removed - the preview card is sufficient feedback } } } diff --git a/App/Composition/CompositionViewController.swift b/App/Composition/CompositionViewController.swift index 844aa1eeb..81f2fef58 100644 --- a/App/Composition/CompositionViewController.swift +++ b/App/Composition/CompositionViewController.swift @@ -137,7 +137,7 @@ final class CompositionViewController: ViewController { // For edits, show existing attachment info if available if let editDraft = draft as? EditReplyDraft { if let existingFilename = editDraft.existingAttachmentFilename { - showAttachmentEditView(filename: existingFilename, filesize: editDraft.existingAttachmentFilesize) + showAttachmentEditView(filename: existingFilename, filesize: editDraft.existingAttachmentFilesize, image: editDraft.existingAttachmentImage) return } } @@ -168,11 +168,11 @@ final class CompositionViewController: ViewController { } } - private func showAttachmentEditView(filename: String, filesize: String?) { + private func showAttachmentEditView(filename: String, filesize: String?, image: UIImage? = nil) { attachmentPreviewView.isHidden = true attachmentPreviewHeightConstraint.constant = 0 - attachmentEditView.configure(filename: filename, filesize: filesize) + attachmentEditView.configure(filename: filename, filesize: filesize, image: image) attachmentEditView.isHidden = false attachmentEditHeightConstraint.constant = 120 diff --git a/App/Composition/ForumAttachment.swift b/App/Composition/ForumAttachment.swift index 1bb4a1425..61facac2f 100644 --- a/App/Composition/ForumAttachment.swift +++ b/App/Composition/ForumAttachment.swift @@ -175,6 +175,23 @@ final class ForumAttachment: NSObject, NSCoding { } } +extension ForumAttachment.ValidationError { + var localizedDescription: String { + switch self { + case .fileTooLarge(let actualSize, let maxSize): + let formatter = ByteCountFormatter() + formatter.countStyle = .file + return "File size (\(formatter.string(fromByteCount: Int64(actualSize)))) exceeds maximum (\(formatter.string(fromByteCount: Int64(maxSize))))" + case .dimensionsTooLarge(let width, let height, let maxDimension): + return "Image dimensions (\(width)×\(height)) exceed maximum (\(maxDimension)×\(maxDimension))" + case .unsupportedFormat: + return "Unsupported image format. Supported formats: GIF, JPEG, PNG" + case .imageDataConversionFailed: + return "Failed to process image data" + } + } +} + private extension UIImage { var hasAlpha: Bool { guard let alphaInfo = cgImage?.alphaInfo else { return false } diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index e1f13d5ed..5e7a1450b 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -420,6 +420,7 @@ final class EditReplyDraft: NSObject, ReplyDraft { var text: NSAttributedString? var forumAttachment: ForumAttachment? var existingAttachmentInfo: (id: String, filename: String)? + var existingAttachmentImage: UIImage? var shouldDeleteAttachment = false var attachmentAction: AttachmentAction { @@ -452,6 +453,9 @@ final class EditReplyDraft: NSObject, ReplyDraft { let attachmentFilename = coder.decodeObject(of: NSString.self, forKey: Keys.attachmentFilename) as? String { self.existingAttachmentInfo = (id: attachmentID, filename: attachmentFilename) } + if let imageData = coder.decodeObject(of: NSData.self, forKey: Keys.attachmentImageData) as? Data { + self.existingAttachmentImage = UIImage(data: imageData) + } self.shouldDeleteAttachment = coder.decodeBool(forKey: Keys.shouldDeleteAttachment) } @@ -462,6 +466,9 @@ final class EditReplyDraft: NSObject, ReplyDraft { coder.encode(existingAttachmentInfo.id as NSString, forKey: Keys.attachmentID) coder.encode(existingAttachmentInfo.filename as NSString, forKey: Keys.attachmentFilename) } + if let imageData = existingAttachmentImage?.pngData() { + coder.encode(imageData as NSData, forKey: Keys.attachmentImageData) + } coder.encode(shouldDeleteAttachment, forKey: Keys.shouldDeleteAttachment) } @@ -470,12 +477,15 @@ final class EditReplyDraft: NSObject, ReplyDraft { static let text = "text" static let attachmentID = "attachmentID" static let attachmentFilename = "attachmentFilename" + static let attachmentImageData = "attachmentImageData" static let shouldDeleteAttachment = "shouldDeleteAttachment" } var thread: AwfulThread { - // TODO can we assume an edited post always has a thread? - return post.thread! + guard let thread = post.thread else { + fatalError("EditReplyDraft requires post to have an associated thread") + } + return thread } var title: String { @@ -504,7 +514,7 @@ extension NewReplyDraft: SubmittableDraft { maxDimension: limits.maxDimension ) { let error = NSError(domain: "Awful", code: 0, userInfo: [ - NSLocalizedDescriptionKey: self.validationErrorMessage(for: validationError) + NSLocalizedDescriptionKey: validationError.localizedDescription ]) completion(error) return @@ -521,21 +531,6 @@ extension NewReplyDraft: SubmittableDraft { } } } - - private func validationErrorMessage(for error: ForumAttachment.ValidationError) -> String { - switch error { - case .fileTooLarge(let actualSize, let maxSize): - let actualMB = Double(actualSize) / 1_048_576 - let maxMB = Double(maxSize) / 1_048_576 - return String(format: "Image is too large (%.1f MB). Maximum size is %.1f MB.", actualMB, maxMB) - case .dimensionsTooLarge(let width, let height, let maxDimension): - return "Image dimensions (\(width)×\(height)) exceed maximum (\(maxDimension)×\(maxDimension))." - case .unsupportedFormat: - return "Unsupported image format. Supported formats: GIF, JPEG, PNG" - case .imageDataConversionFailed: - return "Failed to convert image data" - } - } } extension EditReplyDraft: SubmittableDraft { diff --git a/App/View Controllers/Posts/PostsPageViewController.swift b/App/View Controllers/Posts/PostsPageViewController.swift index 0ac78962b..34e55a0c6 100644 --- a/App/View Controllers/Posts/PostsPageViewController.swift +++ b/App/View Controllers/Posts/PostsPageViewController.swift @@ -1253,10 +1253,21 @@ final class PostsPageViewController: ViewController { let attachmentInfo = try? await ForumsClient.shared.findAttachmentInfo(for: selectedPost!) let replyWorkspace = ReplyWorkspace(post: selectedPost!, bbcode: text) - // Set attachment info before creating the composition view controller + // Set attachment info and fetch image before creating the composition view controller if let attachmentInfo = attachmentInfo, let editDraft = replyWorkspace.draft as? EditReplyDraft { editDraft.existingAttachmentInfo = attachmentInfo + + // Fetch the attachment image + do { + let imageData = try await ForumsClient.shared.fetchAttachmentImage(postID: selectedPost!.postID) + if let image = UIImage(data: imageData) { + editDraft.existingAttachmentImage = image + } + } catch { + logger.error("Failed to fetch attachment image for edit: \(error)") + // Continue anyway - AttachmentEditView will show generic icon + } } self.replyWorkspace = replyWorkspace diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index 80895cc53..e73d954ad 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -884,6 +884,8 @@ public final class ForumsClient { return limits } + // Fallback to SA default attachment limits (2 MB file size, 4096px max dimension) + // These values match ForumAttachment.maxFileSize and ForumAttachment.maxDimension return (maxFileSize: 2_097_152, maxDimension: 4096) } From d5c7e3d28a25ce6aed31ccc5461daca15be6b6ec Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 14:53:41 +1100 Subject: [PATCH 03/13] Second refactor pass --- App/Posts/ReplyWorkspace.swift | 1 - App/Resources/Localizable.xcstrings | 14 ++++++++++- .../Posts/PostsPageViewController.swift | 24 +++++++++++++++---- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index 5e7a1450b..b847174b9 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -370,7 +370,6 @@ final class NewReplyDraft: NSObject, ReplyDraft { let thread: AwfulThread var text: NSAttributedString? var forumAttachment: ForumAttachment? - var existingAttachmentInfo: (id: String, filename: String)? var shouldDeleteAttachment = false init(thread: AwfulThread, text: NSAttributedString? = nil) { diff --git a/App/Resources/Localizable.xcstrings b/App/Resources/Localizable.xcstrings index 0324b5296..786d05083 100644 --- a/App/Resources/Localizable.xcstrings +++ b/App/Resources/Localizable.xcstrings @@ -716,6 +716,18 @@ } } }, + "posts-page.error.attachment-preview-failed" : { + "comment" : "Message shown when attachment image preview fails to load.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Could not load attachment preview" + } + } + } + }, "posts-page.error.could-not-edit-post" : { "comment" : "Title of alert shown when failing to edit post.", "extractionState" : "migrated", @@ -1263,4 +1275,4 @@ } }, "version" : "1.0" -} \ No newline at end of file +} diff --git a/App/View Controllers/Posts/PostsPageViewController.swift b/App/View Controllers/Posts/PostsPageViewController.swift index 34e55a0c6..75d430ad7 100644 --- a/App/View Controllers/Posts/PostsPageViewController.swift +++ b/App/View Controllers/Posts/PostsPageViewController.swift @@ -1249,9 +1249,14 @@ final class PostsPageViewController: ViewController { func presentNewReplyWorkspace() { Task { do { - let text = try await ForumsClient.shared.findBBcodeContents(of: selectedPost!) - let attachmentInfo = try? await ForumsClient.shared.findAttachmentInfo(for: selectedPost!) - let replyWorkspace = ReplyWorkspace(post: selectedPost!, bbcode: text) + guard let selectedPost = selectedPost else { + logger.error("Cannot edit: no post selected") + return + } + + let text = try await ForumsClient.shared.findBBcodeContents(of: selectedPost) + let attachmentInfo = try? await ForumsClient.shared.findAttachmentInfo(for: selectedPost) + let replyWorkspace = ReplyWorkspace(post: selectedPost, bbcode: text) // Set attachment info and fetch image before creating the composition view controller if let attachmentInfo = attachmentInfo, @@ -1260,12 +1265,23 @@ final class PostsPageViewController: ViewController { // Fetch the attachment image do { - let imageData = try await ForumsClient.shared.fetchAttachmentImage(postID: selectedPost!.postID) + let imageData = try await ForumsClient.shared.fetchAttachmentImage(postID: selectedPost.postID) if let image = UIImage(data: imageData) { editDraft.existingAttachmentImage = image } } catch { logger.error("Failed to fetch attachment image for edit: \(error)") + // Show a toast notification to inform the user + let alert = UIAlertController( + title: nil, + message: LocalizedString("posts-page.error.attachment-preview-failed"), + preferredStyle: .alert + ) + present(alert, animated: true) + // Auto-dismiss after 2 seconds + DispatchQueue.main.asyncAfter(deadline: .now() + 2.0) { + alert.dismiss(animated: true) + } // Continue anyway - AttachmentEditView will show generic icon } } From 7352acd5396fc4bc0554491fcc7e12ecf3517eaa Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:05:08 +1100 Subject: [PATCH 04/13] Another refactor. --- App/Composition/AttachmentPreviewView.swift | 4 ++ App/Composition/CompositionMenuTree.swift | 6 ++- App/Composition/ForumAttachment.swift | 53 +++++++++++++++---- App/Resources/RenderView.js | 4 +- .../URLRequest+MultipartFormData.swift | 44 +++++++++++---- 5 files changed, 88 insertions(+), 23 deletions(-) diff --git a/App/Composition/AttachmentPreviewView.swift b/App/Composition/AttachmentPreviewView.swift index 315ef715f..b8c036b0c 100644 --- a/App/Composition/AttachmentPreviewView.swift +++ b/App/Composition/AttachmentPreviewView.swift @@ -2,8 +2,11 @@ // // Copyright 2025 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app +import os import UIKit +private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "AttachmentPreviewView") + /// A card-style view that shows a preview of an attached image with options to remove it. final class AttachmentPreviewView: UIView { @@ -121,6 +124,7 @@ final class AttachmentPreviewView: UIView { let sizeString = formatter.string(fromByteCount: Int64(data.count)) detailLabel.text = "\(width) × \(height) • \(sizeString)" } catch { + logger.error("Failed to get image data for attachment preview: \(error.localizedDescription)") detailLabel.text = "\(width) × \(height)" } } diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 92176d0c7..f4fde3dc6 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -368,8 +368,10 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont DispatchQueue.global(qos: .userInitiated).async { [weak self] in guard let self = self else { return } - let originalAttachment = ForumAttachment(image: image, photoAssetIdentifier: self.pendingImageAssetIdentifier) - let resizedAttachment = originalAttachment.resized() + let resizedAttachment = autoreleasepool { () -> ForumAttachment? in + let originalAttachment = ForumAttachment(image: image, photoAssetIdentifier: self.pendingImageAssetIdentifier) + return originalAttachment.resized() + } DispatchQueue.main.async { self.pendingImage = nil diff --git a/App/Composition/ForumAttachment.swift b/App/Composition/ForumAttachment.swift index 61facac2f..333880225 100644 --- a/App/Composition/ForumAttachment.swift +++ b/App/Composition/ForumAttachment.swift @@ -2,9 +2,12 @@ // // Copyright 2025 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app +import os import Photos import UIKit +private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "ForumAttachment") + final class ForumAttachment: NSObject, NSCoding { static let maxFileSize = 2_097_152 @@ -38,11 +41,23 @@ final class ForumAttachment: NSObject, NSCoding { options.isSynchronous = true options.deliveryMode = .highQualityFormat var resultImage: UIImage? - PHImageManager.default().requestImage(for: asset, targetSize: PHImageManagerMaximumSize, contentMode: .default, options: options) { image, _ in + var requestError: Error? + PHImageManager.default().requestImage(for: asset, targetSize: PHImageManagerMaximumSize, contentMode: .default, options: options) { image, info in resultImage = image + if let error = info?[PHImageErrorKey] as? Error { + requestError = error + } + } + + if let error = requestError { + logger.error("Failed to load image from photo library asset: \(error.localizedDescription)") + } else if resultImage == nil { + logger.warning("Photo library request returned nil image for asset: \(photoAssetIdentifier as String)") } + self.image = resultImage } else { + logger.warning("Photo asset not found for identifier: \(photoAssetIdentifier as String)") self.image = nil } } else if let imageData = coder.decodeObject(of: NSData.self, forKey: CodingKeys.imageData.rawValue) { @@ -142,8 +157,28 @@ final class ForumAttachment: NSObject, NSCoding { targetHeight = Int(CGFloat(originalHeight) * ratio) } + if let compressed = compressImage( + originalImage, + targetWidth: targetWidth, + targetHeight: targetHeight, + maxFileSize: effectiveMaxFileSize + ) { + return ForumAttachment(image: compressed, photoAssetIdentifier: photoAssetIdentifier) + } + + return nil + } + + private func compressImage( + _ originalImage: UIImage, + targetWidth: Int, + targetHeight: Int, + maxFileSize: Int + ) -> UIImage? { var compressionQuality: CGFloat = 0.9 - var resizedImage = originalImage.resized(to: CGSize(width: targetWidth, height: targetHeight)) + var currentWidth = targetWidth + var currentHeight = targetHeight + var resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) while compressionQuality > 0.1 { let hasAlpha = resizedImage.hasAlpha @@ -155,17 +190,17 @@ final class ForumAttachment: NSObject, NSCoding { data = resizedImage.jpegData(compressionQuality: compressionQuality) } - if let imageData = data, imageData.count <= effectiveMaxFileSize { - return ForumAttachment(image: resizedImage, photoAssetIdentifier: photoAssetIdentifier) + if let imageData = data, imageData.count <= maxFileSize { + return resizedImage } if hasAlpha { - let newWidth = Int(CGFloat(targetWidth) * 0.9) - let newHeight = Int(CGFloat(targetHeight) * 0.9) + let newWidth = Int(CGFloat(currentWidth) * 0.9) + let newHeight = Int(CGFloat(currentHeight) * 0.9) if newWidth < 100 || newHeight < 100 { break } - targetWidth = newWidth - targetHeight = newHeight - resizedImage = originalImage.resized(to: CGSize(width: targetWidth, height: targetHeight)) + currentWidth = newWidth + currentHeight = newHeight + resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) } else { compressionQuality -= 0.1 } diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 1b6b19bb4..533499048 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -957,9 +957,9 @@ Awful.embedGfycat(); // Load attachment images asynchronously Awful.loadAttachmentImages = function() { var attachmentImages = document.querySelectorAll('img[data-awful-attachment-postid]'); - attachmentImages.forEach(function(img) { + attachmentImages.forEach(function(img, index) { var postID = img.getAttribute('data-awful-attachment-postid'); - var id = 'attachment-' + Date.now() + '-' + Math.random(); + var id = 'attachment-' + postID + '-' + index + '-' + Date.now(); img.setAttribute('data-awful-attachment-id', id); window.webkit.messageHandlers.fetchAttachmentImage.postMessage({ id: id, diff --git a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift index c3f7d2823..17419af9e 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift @@ -34,17 +34,26 @@ extension URLRequest { for (rawName, rawValue) in parameters { if !body.isEmpty { - body.append("\r\n".data(using: .utf8)!) + guard let newline = "\r\n".data(using: .utf8) else { + throw EncodingError("UTF-8 encoding failed") + } + body.append(newline) } - body.append("--\(boundary)\r\n".data(using: .utf8)!) + guard let boundaryData = "--\(boundary)\r\n".data(using: .utf8) else { + throw EncodingError("UTF-8 encoding failed") + } + body.append(boundaryData) guard rawName.canBeConverted(to: encoding), let disposition = "Content-Disposition: form-data; name=\"\(rawName)\"\r\n".data(using: .utf8) else { throw EncodingError("name") } body.append(disposition) - body.append("\r\n".data(using: .utf8)!) + guard let headerEnd = "\r\n".data(using: .utf8) else { + throw EncodingError("UTF-8 encoding failed") + } + body.append(headerEnd) guard let value = rawValue.data(using: encoding) else { throw EncodingError("value") @@ -53,7 +62,10 @@ extension URLRequest { body.append(value) } - body.append("\r\n--\(boundary)--\r\n".data(using: .utf8)!) + guard let finalBoundary = "\r\n--\(boundary)--\r\n".data(using: .utf8) else { + throw EncodingError("UTF-8 encoding failed") + } + body.append(finalBoundary) return body }() @@ -78,7 +90,10 @@ extension URLRequest { let boundary = String(contentType[boundaryRange.upperBound...]).trimmingCharacters(in: .whitespaces) - let lastBoundary = "--\(boundary)--\r\n".data(using: .utf8)! + guard let lastBoundary = "--\(boundary)--\r\n".data(using: .utf8) else { + throw EncodingError("UTF-8 encoding failed") + } + if existingBody.suffix(lastBoundary.count) == lastBoundary { existingBody.removeLast(lastBoundary.count) } else { @@ -86,12 +101,21 @@ extension URLRequest { } var filePartData = Data() - filePartData.append("\r\n--\(boundary)\r\n".data(using: .utf8)!) - filePartData.append("Content-Disposition: form-data; name=\"\(name)\"; filename=\"\(filename)\"\r\n".data(using: .utf8)!) - filePartData.append("Content-Type: \(mimeType)\r\n".data(using: .utf8)!) - filePartData.append("\r\n".data(using: .utf8)!) + + guard let newline = "\r\n--\(boundary)\r\n".data(using: .utf8), + let disposition = "Content-Disposition: form-data; name=\"\(name)\"; filename=\"\(filename)\"\r\n".data(using: .utf8), + let contentTypeData = "Content-Type: \(mimeType)\r\n".data(using: .utf8), + let headerEnd = "\r\n".data(using: .utf8), + let finalBoundary = "\r\n--\(boundary)--\r\n".data(using: .utf8) else { + throw EncodingError("UTF-8 encoding failed") + } + + filePartData.append(newline) + filePartData.append(disposition) + filePartData.append(contentTypeData) + filePartData.append(headerEnd) filePartData.append(data) - filePartData.append("\r\n--\(boundary)--\r\n".data(using: .utf8)!) + filePartData.append(finalBoundary) existingBody.append(filePartData) httpBody = existingBody From 5a960175243badb3a5e3f4b6c7968ed0f9132774 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:12:02 +1100 Subject: [PATCH 05/13] Final refactor? --- App/Composition/CompositionMenuTree.swift | 186 +++++++++--------- .../CompositionViewController.swift | 34 +--- App/Composition/ForumAttachment.swift | 24 ++- App/Posts/ReplyWorkspace.swift | 20 +- 4 files changed, 137 insertions(+), 127 deletions(-) diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index f4fde3dc6..017b98846 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -110,105 +110,113 @@ final class CompositionMenuTree: NSObject { private func authenticateWithImgur() { guard let viewController = textView.nearestViewController else { return } - - // Show an alert to explain why authentication is needed + showAuthenticationPrompt(in: viewController) + } + + private func showAuthenticationPrompt(in viewController: UIViewController) { let alert = UIAlertController( title: "Imgur Authentication Required", message: "You've enabled Imgur Account uploads in settings. To upload images with your account, you'll need to log in to Imgur.", preferredStyle: .alert ) - - alert.addAction(UIAlertAction(title: "Log In", style: .default) { _ in - // Show loading indicator - let loadingAlert = UIAlertController( - title: "Connecting to Imgur", - message: "Please wait...", - preferredStyle: .alert - ) - viewController.present(loadingAlert, animated: true) - - ImgurAuthManager.shared.authenticate(from: viewController) { success in - // Dismiss loading indicator - DispatchQueue.main.async { - loadingAlert.dismiss(animated: true) { - if success { - // If authentication was successful, continue with the upload - // Show a success message - let successAlert = UIAlertController( - title: "Successfully Logged In", - message: "You're now logged in to Imgur and can upload images with your account.", - preferredStyle: .alert - ) - - successAlert.addAction(UIAlertAction(title: "Continue", style: .default) { _ in - // Continue with image picker after successful authentication - self.showImagePicker(.photoLibrary) - }) - - viewController.present(successAlert, animated: true) - } else { - // Check if it's a rate limiting issue (check logs from ImgurAuthManager) - let isRateLimited = UserDefaults.standard.bool(forKey: ImgurAuthManager.DefaultsKeys.rateLimited) - - if isRateLimited { - // Show specific rate limiting error - let rateLimitAlert = UIAlertController( - title: "Imgur Rate Limit Exceeded", - message: "Imgur's API is currently rate limited. You can try again later or use anonymous uploads for now.", - preferredStyle: .alert - ) - - rateLimitAlert.addAction(UIAlertAction(title: "Use Anonymous Uploads", style: .default) { _ in - // Switch to anonymous uploads for this session - self.imgurUploadMode = .anonymous - // Continue with image picker - self.showImagePicker(.photoLibrary) - }) - - rateLimitAlert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) - - viewController.present(rateLimitAlert, animated: true) - } else { - // General authentication failure - let failureAlert = UIAlertController( - title: "Authentication Failed", - message: "Could not log in to Imgur. You can try again or choose anonymous uploads in settings.", - preferredStyle: .alert - ) - - failureAlert.addAction(UIAlertAction(title: "Try Again", style: .default) { _ in - // Try authentication again - self.authenticateWithImgur() - }) - - failureAlert.addAction(UIAlertAction(title: "Use Anonymous Upload", style: .default) { _ in - // Use anonymous uploads for this session - self.imgurUploadMode = .anonymous - // Continue with image picker - self.showImagePicker(.photoLibrary) - }) - - failureAlert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) - - viewController.present(failureAlert, animated: true) - } - } - } + + alert.addAction(UIAlertAction(title: "Log In", style: .default) { [weak self] _ in + self?.performAuthentication(in: viewController) + }) + + alert.addAction(UIAlertAction(title: "Use Anonymous Upload", style: .default) { [weak self] _ in + self?.switchToAnonymousUploads() + }) + + alert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) + + viewController.present(alert, animated: true) + } + + private func performAuthentication(in viewController: UIViewController) { + let loadingAlert = UIAlertController( + title: "Connecting to Imgur", + message: "Please wait...", + preferredStyle: .alert + ) + viewController.present(loadingAlert, animated: true) + + ImgurAuthManager.shared.authenticate(from: viewController) { [weak self] success in + DispatchQueue.main.async { + loadingAlert.dismiss(animated: true) { + self?.handleAuthenticationResult(success, in: viewController) } } + } + } + + private func handleAuthenticationResult(_ success: Bool, in viewController: UIViewController) { + if success { + presentAuthenticationSuccessAlert(in: viewController) + } else { + let isRateLimited = UserDefaults.standard.bool(forKey: ImgurAuthManager.DefaultsKeys.rateLimited) + if isRateLimited { + presentRateLimitAlert(in: viewController) + } else { + presentAuthenticationFailureAlert(in: viewController) + } + } + } + + private func presentAuthenticationSuccessAlert(in viewController: UIViewController) { + let alert = UIAlertController( + title: "Successfully Logged In", + message: "You're now logged in to Imgur and can upload images with your account.", + preferredStyle: .alert + ) + + alert.addAction(UIAlertAction(title: "Continue", style: .default) { [weak self] _ in + self?.showImagePicker(.photoLibrary) }) - - alert.addAction(UIAlertAction(title: "Use Anonymous Upload", style: .default) { _ in - // Use anonymous uploads just for this session - self.imgurUploadMode = .anonymous - // Show image picker with anonymous uploads - self.showImagePicker(.photoLibrary) + + viewController.present(alert, animated: true) + } + + private func presentRateLimitAlert(in viewController: UIViewController) { + let alert = UIAlertController( + title: "Imgur Rate Limit Exceeded", + message: "Imgur's API is currently rate limited. You can try again later or use anonymous uploads for now.", + preferredStyle: .alert + ) + + alert.addAction(UIAlertAction(title: "Use Anonymous Uploads", style: .default) { [weak self] _ in + self?.switchToAnonymousUploads() }) - + alert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) - + viewController.present(alert, animated: true) } + + private func presentAuthenticationFailureAlert(in viewController: UIViewController) { + let alert = UIAlertController( + title: "Authentication Failed", + message: "Could not log in to Imgur. You can try again or choose anonymous uploads in settings.", + preferredStyle: .alert + ) + + alert.addAction(UIAlertAction(title: "Try Again", style: .default) { [weak self] _ in + self?.authenticateWithImgur() + }) + + alert.addAction(UIAlertAction(title: "Use Anonymous Upload", style: .default) { [weak self] _ in + self?.switchToAnonymousUploads() + }) + + alert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) + + viewController.present(alert, animated: true) + } + + private func switchToAnonymousUploads() { + imgurUploadMode = .anonymous + showImagePicker(.photoLibrary) + } func insertImage(_ image: UIImage, withAssetIdentifier assetID: String? = nil) { // Inserting the image changes our font and text color, so save those now and restore those later. @@ -431,10 +439,6 @@ fileprivate struct MenuItem { init(title: String, action: @escaping (CompositionMenuTree) -> Void) { self.init(title: title, action: action, enabled: { true }) } - - func psItem(_ tree: CompositionMenuTree) { - return - } } fileprivate let rootItems = [ diff --git a/App/Composition/CompositionViewController.swift b/App/Composition/CompositionViewController.swift index 81f2fef58..9a046f86c 100644 --- a/App/Composition/CompositionViewController.swift +++ b/App/Composition/CompositionViewController.swift @@ -118,14 +118,7 @@ final class CompositionViewController: ViewController { attachmentEditView.isHidden = true attachmentEditHeightConstraint.constant = 0 - // Update text view constraint to anchor to preview view - textViewTopConstraint.isActive = false - textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) - textViewTopConstraint.isActive = true - - UIView.animate(withDuration: 0.3) { - self.view.layoutIfNeeded() - } + updateTextViewConstraint(anchoredTo: attachmentPreviewView.bottomAnchor) } private func updateAttachmentPreview() { @@ -158,14 +151,7 @@ final class CompositionViewController: ViewController { attachmentPreviewView.isHidden = false attachmentPreviewHeightConstraint.constant = 84 - // Update text view constraint to anchor to preview view - textViewTopConstraint.isActive = false - textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) - textViewTopConstraint.isActive = true - - UIView.animate(withDuration: 0.3) { - self.view.layoutIfNeeded() - } + updateTextViewConstraint(anchoredTo: attachmentPreviewView.bottomAnchor) } private func showAttachmentEditView(filename: String, filesize: String?, image: UIImage? = nil) { @@ -176,14 +162,7 @@ final class CompositionViewController: ViewController { attachmentEditView.isHidden = false attachmentEditHeightConstraint.constant = 120 - // Update text view constraint to anchor to edit view - textViewTopConstraint.isActive = false - textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentEditView.bottomAnchor, constant: 8) - textViewTopConstraint.isActive = true - - UIView.animate(withDuration: 0.3) { - self.view.layoutIfNeeded() - } + updateTextViewConstraint(anchoredTo: attachmentEditView.bottomAnchor) } private func hideAllAttachmentViews() { @@ -193,9 +172,12 @@ final class CompositionViewController: ViewController { attachmentEditView.isHidden = true attachmentEditHeightConstraint.constant = 0 - // Reset text view constraint to anchor to preview view (at height 0) + updateTextViewConstraint(anchoredTo: attachmentPreviewView.bottomAnchor) + } + + private func updateTextViewConstraint(anchoredTo anchor: NSLayoutYAxisAnchor, constant: CGFloat = 8) { textViewTopConstraint.isActive = false - textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) + textViewTopConstraint = _textView.topAnchor.constraint(equalTo: anchor, constant: constant) textViewTopConstraint.isActive = true UIView.animate(withDuration: 0.3) { diff --git a/App/Composition/ForumAttachment.swift b/App/Composition/ForumAttachment.swift index 333880225..d5d660484 100644 --- a/App/Composition/ForumAttachment.swift +++ b/App/Composition/ForumAttachment.swift @@ -14,6 +14,12 @@ final class ForumAttachment: NSObject, NSCoding { static let maxDimension = 4096 static let supportedExtensions = ["gif", "jpg", "jpeg", "png"] + private static let defaultCompressionQuality: CGFloat = 0.9 + private static let compressionQualityDecrement: CGFloat = 0.1 + private static let minCompressionQuality: CGFloat = 0.1 + private static let dimensionScaleFactor: CGFloat = 0.9 + private static let minimumDimension = 100 + let image: UIImage? let photoAssetIdentifier: String? private(set) var validationError: ValidationError? @@ -131,7 +137,7 @@ final class ForumAttachment: NSObject, NSCoding { if hasAlpha, let pngData = image.pngData() { return (pngData, "photo-\(timestamp).png", "image/png") - } else if let jpegData = image.jpegData(compressionQuality: 0.9) { + } else if let jpegData = image.jpegData(compressionQuality: Self.defaultCompressionQuality) { return (jpegData, "photo-\(timestamp).jpg", "image/jpeg") } else { throw ValidationError.imageDataConversionFailed @@ -175,12 +181,12 @@ final class ForumAttachment: NSObject, NSCoding { targetHeight: Int, maxFileSize: Int ) -> UIImage? { - var compressionQuality: CGFloat = 0.9 + var compressionQuality = Self.defaultCompressionQuality var currentWidth = targetWidth var currentHeight = targetHeight var resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) - while compressionQuality > 0.1 { + while compressionQuality > Self.minCompressionQuality { let hasAlpha = resizedImage.hasAlpha let data: Data? @@ -195,14 +201,14 @@ final class ForumAttachment: NSObject, NSCoding { } if hasAlpha { - let newWidth = Int(CGFloat(currentWidth) * 0.9) - let newHeight = Int(CGFloat(currentHeight) * 0.9) - if newWidth < 100 || newHeight < 100 { break } + let newWidth = Int(CGFloat(currentWidth) * Self.dimensionScaleFactor) + let newHeight = Int(CGFloat(currentHeight) * Self.dimensionScaleFactor) + if newWidth < Self.minimumDimension || newHeight < Self.minimumDimension { break } currentWidth = newWidth currentHeight = newHeight resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) } else { - compressionQuality -= 0.1 + compressionQuality -= Self.compressionQualityDecrement } } @@ -228,9 +234,11 @@ extension ForumAttachment.ValidationError { } private extension UIImage { + private static let alphaInfoTypes: Set = [.first, .last, .premultipliedFirst, .premultipliedLast] + var hasAlpha: Bool { guard let alphaInfo = cgImage?.alphaInfo else { return false } - return alphaInfo == .first || alphaInfo == .last || alphaInfo == .premultipliedFirst || alphaInfo == .premultipliedLast + return Self.alphaInfoTypes.contains(alphaInfo) } func resized(to targetSize: CGSize) -> UIImage { diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index b847174b9..448b3107e 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -498,7 +498,15 @@ final class EditReplyDraft: NSObject, ReplyDraft { extension NewReplyDraft: SubmittableDraft { func submit(_ completion: @escaping (Error?) -> Void) -> Progress { - return uploadImages(attachedTo: text!) { [unowned self] plainText, error in + guard let text = text else { + let error = NSError(domain: "Awful", code: 0, userInfo: [ + NSLocalizedDescriptionKey: "Post text cannot be empty" + ]) + completion(error) + return Progress(totalUnitCount: 1) + } + + return uploadImages(attachedTo: text) { [unowned self] plainText, error in if let error = error { completion(error) } else { @@ -534,7 +542,15 @@ extension NewReplyDraft: SubmittableDraft { extension EditReplyDraft: SubmittableDraft { func submit(_ completion: @escaping (Error?) -> Void) -> Progress { - return uploadImages(attachedTo: text!) { [unowned self] plainText, error in + guard let text = text else { + let error = NSError(domain: "Awful", code: 0, userInfo: [ + NSLocalizedDescriptionKey: "Post text cannot be empty" + ]) + completion(error) + return Progress(totalUnitCount: 1) + } + + return uploadImages(attachedTo: text) { [unowned self] plainText, error in if let error = error { completion(error) } else { From bdef93fbccfc2fc28981d944648cca7ee5eaabb5 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:20:14 +1100 Subject: [PATCH 06/13] One more refactor --- App/Composition/CompositionMenuTree.swift | 34 +++------- .../CompositionViewController.swift | 68 +++++++++++-------- App/Composition/ForumAttachment.swift | 52 ++++++++++---- App/Posts/ReplyWorkspace.swift | 1 - .../AwfulCore/Networking/ForumsClient.swift | 48 +++++++++++++ 5 files changed, 139 insertions(+), 64 deletions(-) diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 017b98846..98941481d 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -323,7 +323,13 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont let attachment = ForumAttachment(image: image, photoAssetIdentifier: pendingImageAssetIdentifier) if let error = attachment.validationError { - if canResize(error) { + // Check if the error can be fixed by resizing + let canResize = switch error { + case .fileTooLarge, .dimensionsTooLarge: true + default: false + } + + if canResize { let alert = UIAlertController( title: "Attachment Too Large", message: "\(error.localizedDescription)\n\nWould you like to automatically resize the image to fit?", @@ -357,15 +363,6 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont onAttachmentChanged?() } - private func canResize(_ error: ForumAttachment.ValidationError) -> Bool { - switch error { - case .fileTooLarge, .dimensionsTooLarge: - return true - default: - return false - } - } - private func resizeAndAttachPendingImage() { guard let image = pendingImage else { return } @@ -386,8 +383,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont self.pendingImageAssetIdentifier = nil guard let resizedAttachment = resizedAttachment else { - // Remove placeholder on failure - self.draft?.forumAttachment = nil + // Clear placeholder on failure self.onAttachmentChanged?() let alert = UIAlertController( @@ -400,8 +396,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } if let error = resizedAttachment.validationError { - // Remove placeholder on validation failure - self.draft?.forumAttachment = nil + // Clear placeholder on validation failure self.onAttachmentChanged?() let alert = UIAlertController( @@ -449,19 +444,12 @@ fileprivate let rootItems = [ tree.showSubmenu(URLItems) } }), - /** - Temporarily disabling the menu items that attempt image uploads. This is a bandaid fix and no imgur uploading code is being removed from the app at this time. - TODO: Re-enable these menu items as part of a proper imgur replacement update. (imgur is deleting anonymous inactive images) - - original line: MenuItem(title: "[img]", action: { $0.showSubmenu(imageItems) }), - */ MenuItem(title: "[img]", action: { tree in - // Show the image submenu if either: - // 1. Imgur uploads are enabled in settings, OR - // 2. The draft is a NewReplyDraft (for forum attachments) + // Show the image submenu if Imgur uploads are enabled or forum attachments are available if tree.imgurUploadsEnabled || tree.draft is NewReplyDraft { tree.showSubmenu(imageItems(tree: tree)) } else { + // Fallback: paste image URL from clipboard or wrap selection if UIPasteboard.general.coercedURL == nil { linkifySelection(tree) } else { diff --git a/App/Composition/CompositionViewController.swift b/App/Composition/CompositionViewController.swift index 9a046f86c..63880e51b 100644 --- a/App/Composition/CompositionViewController.swift +++ b/App/Composition/CompositionViewController.swift @@ -10,6 +10,12 @@ final class CompositionViewController: ViewController { @FoilDefaultStorage(Settings.enableHaptics) private var enableHaptics + private enum AttachmentViewLayout { + static let previewHeight: CGFloat = 84 + static let editHeight: CGFloat = 120 + static let spacing: CGFloat = 8 + } + override init(nibName: String?, bundle: Bundle?) { super.init(nibName: nil, bundle: nil) restorationClass = type(of: self) @@ -76,7 +82,7 @@ final class CompositionViewController: ViewController { attachmentEditHeightConstraint = attachmentEditView.heightAnchor.constraint(equalToConstant: 0) // Default: text view top anchors to preview view bottom (which starts at height 0) - textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: 8) + textViewTopConstraint = _textView.topAnchor.constraint(equalTo: attachmentPreviewView.bottomAnchor, constant: AttachmentViewLayout.spacing) NSLayoutConstraint.activate([ attachmentPreviewView.topAnchor.constraint(equalTo: containerView.safeAreaLayoutGuide.topAnchor, constant: 8), @@ -112,13 +118,11 @@ final class CompositionViewController: ViewController { func showResizingPlaceholder() { attachmentPreviewView.showResizingPlaceholder() - attachmentPreviewView.isHidden = false - attachmentPreviewHeightConstraint.constant = 84 - - attachmentEditView.isHidden = true - attachmentEditHeightConstraint.constant = 0 - - updateTextViewConstraint(anchoredTo: attachmentPreviewView.bottomAnchor) + updateAttachmentViewVisibility( + showPreview: true, previewHeight: AttachmentViewLayout.previewHeight, + showEdit: false, editHeight: 0, + anchorTextViewTo: attachmentPreviewView.bottomAnchor + ) } private func updateAttachmentPreview() { @@ -144,38 +148,46 @@ final class CompositionViewController: ViewController { } private func showAttachmentPreview(with attachment: ForumAttachment) { - attachmentEditView.isHidden = true - attachmentEditHeightConstraint.constant = 0 - attachmentPreviewView.configure(with: attachment) - attachmentPreviewView.isHidden = false - attachmentPreviewHeightConstraint.constant = 84 - - updateTextViewConstraint(anchoredTo: attachmentPreviewView.bottomAnchor) + updateAttachmentViewVisibility( + showPreview: true, previewHeight: AttachmentViewLayout.previewHeight, + showEdit: false, editHeight: 0, + anchorTextViewTo: attachmentPreviewView.bottomAnchor + ) } private func showAttachmentEditView(filename: String, filesize: String?, image: UIImage? = nil) { - attachmentPreviewView.isHidden = true - attachmentPreviewHeightConstraint.constant = 0 - attachmentEditView.configure(filename: filename, filesize: filesize, image: image) - attachmentEditView.isHidden = false - attachmentEditHeightConstraint.constant = 120 - - updateTextViewConstraint(anchoredTo: attachmentEditView.bottomAnchor) + updateAttachmentViewVisibility( + showPreview: false, previewHeight: 0, + showEdit: true, editHeight: AttachmentViewLayout.editHeight, + anchorTextViewTo: attachmentEditView.bottomAnchor + ) } private func hideAllAttachmentViews() { - attachmentPreviewView.isHidden = true - attachmentPreviewHeightConstraint.constant = 0 + updateAttachmentViewVisibility( + showPreview: false, previewHeight: 0, + showEdit: false, editHeight: 0, + anchorTextViewTo: attachmentPreviewView.bottomAnchor + ) + } - attachmentEditView.isHidden = true - attachmentEditHeightConstraint.constant = 0 + private func updateAttachmentViewVisibility( + showPreview: Bool, previewHeight: CGFloat, + showEdit: Bool, editHeight: CGFloat, + anchorTextViewTo anchor: NSLayoutYAxisAnchor + ) { + attachmentPreviewView.isHidden = !showPreview + attachmentPreviewHeightConstraint.constant = previewHeight + + attachmentEditView.isHidden = !showEdit + attachmentEditHeightConstraint.constant = editHeight - updateTextViewConstraint(anchoredTo: attachmentPreviewView.bottomAnchor) + updateTextViewConstraint(anchoredTo: anchor) } - private func updateTextViewConstraint(anchoredTo anchor: NSLayoutYAxisAnchor, constant: CGFloat = 8) { + private func updateTextViewConstraint(anchoredTo anchor: NSLayoutYAxisAnchor, constant: CGFloat = AttachmentViewLayout.spacing) { textViewTopConstraint.isActive = false textViewTopConstraint = _textView.topAnchor.constraint(equalTo: anchor, constant: constant) textViewTopConstraint.isActive = true diff --git a/App/Composition/ForumAttachment.swift b/App/Composition/ForumAttachment.swift index d5d660484..5d64db02e 100644 --- a/App/Composition/ForumAttachment.swift +++ b/App/Composition/ForumAttachment.swift @@ -8,17 +8,45 @@ import UIKit private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "ForumAttachment") +/** + Represents an image attachment for a forum post with validation and compression capabilities. + + `ForumAttachment` handles image validation, resizing, and compression to meet forum requirements. + It supports both direct image instances and photo library assets, with automatic validation + against file size and dimension limits. + + ## Validation Rules + + Attachments are validated against: + - Maximum file size: 2 MB (2,097,152 bytes) + - Maximum dimensions: 4096×4096 pixels + - Supported formats: GIF, JPEG, PNG + + ## Resizing and Compression + + When an image exceeds limits, `ForumAttachment` can automatically: + 1. Scale down dimensions while maintaining aspect ratio + 2. Apply progressive JPEG compression (for non-transparent images) + 3. Scale down PNG images with transparency until they meet size requirements + + ## State Preservation + + Conforms to `NSCoding` to support UIKit state restoration. Photo library assets are + stored by identifier and reloaded on restoration. + */ final class ForumAttachment: NSObject, NSCoding { static let maxFileSize = 2_097_152 static let maxDimension = 4096 static let supportedExtensions = ["gif", "jpg", "jpeg", "png"] - private static let defaultCompressionQuality: CGFloat = 0.9 - private static let compressionQualityDecrement: CGFloat = 0.1 - private static let minCompressionQuality: CGFloat = 0.1 - private static let dimensionScaleFactor: CGFloat = 0.9 - private static let minimumDimension = 100 + private struct CompressionSettings { + static let defaultQuality: CGFloat = 0.9 + static let qualityDecrement: CGFloat = 0.1 + static let minQuality: CGFloat = 0.1 + static let dimensionScaleFactor: CGFloat = 0.9 + static let minimumDimension = 100 + } let image: UIImage? let photoAssetIdentifier: String? @@ -137,7 +165,7 @@ final class ForumAttachment: NSObject, NSCoding { if hasAlpha, let pngData = image.pngData() { return (pngData, "photo-\(timestamp).png", "image/png") - } else if let jpegData = image.jpegData(compressionQuality: Self.defaultCompressionQuality) { + } else if let jpegData = image.jpegData(compressionQuality: CompressionSettings.defaultQuality) { return (jpegData, "photo-\(timestamp).jpg", "image/jpeg") } else { throw ValidationError.imageDataConversionFailed @@ -181,12 +209,12 @@ final class ForumAttachment: NSObject, NSCoding { targetHeight: Int, maxFileSize: Int ) -> UIImage? { - var compressionQuality = Self.defaultCompressionQuality + var compressionQuality = CompressionSettings.defaultQuality var currentWidth = targetWidth var currentHeight = targetHeight var resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) - while compressionQuality > Self.minCompressionQuality { + while compressionQuality > CompressionSettings.minQuality { let hasAlpha = resizedImage.hasAlpha let data: Data? @@ -201,14 +229,14 @@ final class ForumAttachment: NSObject, NSCoding { } if hasAlpha { - let newWidth = Int(CGFloat(currentWidth) * Self.dimensionScaleFactor) - let newHeight = Int(CGFloat(currentHeight) * Self.dimensionScaleFactor) - if newWidth < Self.minimumDimension || newHeight < Self.minimumDimension { break } + let newWidth = Int(CGFloat(currentWidth) * CompressionSettings.dimensionScaleFactor) + let newHeight = Int(CGFloat(currentHeight) * CompressionSettings.dimensionScaleFactor) + if newWidth < CompressionSettings.minimumDimension || newHeight < CompressionSettings.minimumDimension { break } currentWidth = newWidth currentHeight = newHeight resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) } else { - compressionQuality -= Self.compressionQualityDecrement + compressionQuality -= CompressionSettings.qualityDecrement } } diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index 448b3107e..6dcd3bd43 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -354,7 +354,6 @@ extension ReplyWorkspace: UIObjectRestoration, UIStateRestoring { var text: NSAttributedString? { get set } var title: String { get } var forumAttachment: ForumAttachment? { get set } - var shouldDeleteAttachment: Bool { get set } } @objc protocol SubmittableDraft { diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index e73d954ad..5678c85cb 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -790,6 +790,20 @@ public final class ForumsClient { case post(Post) } + /** + Posts a new reply to a thread. + + - Parameters: + - thread: The thread to reply to. + - bbcode: The BBCode content of the reply. + - attachment: Optional attachment data including the file data, filename, and MIME type. + + - Returns: A tuple containing: + - location: The location of the posted reply (either a specific post or the last post in the thread). + - attachmentLimits: The parsed attachment size and dimension limits from the server, if available. + + - Throws: An error if the reply fails to post or if the thread is closed. + */ public func reply( to thread: AwfulThread, bbcode: String, @@ -870,6 +884,16 @@ public final class ForumsClient { return (location: location, attachmentLimits: parsedLimits) } + /** + Fetches the attachment size and dimension limits for a thread. + + - Parameter thread: The thread to fetch attachment limits for. + + - Returns: A tuple containing the maximum file size (in bytes) and maximum dimension (in pixels). + Falls back to SA default limits (2 MB, 4096px) if parsing fails. + + - Throws: An error if the request fails. + */ public func fetchAttachmentLimits(for thread: AwfulThread) async throws -> (maxFileSize: Int, maxDimension: Int) { let threadID: String = await thread.managedObjectContext!.perform { thread.threadID @@ -926,6 +950,18 @@ public final class ForumsClient { return (maxFileSize: maxFileSize, maxDimension: maxDimension) } + /** + Fetches an attachment image for a post with authentication. + + This method downloads attachment images that require authentication, + typically for posts being edited that already have attachments. + + - Parameter postID: The ID of the post containing the attachment. + + - Returns: The raw image data. + + - Throws: An error if the request fails or returns a non-200 status code. + */ public func fetchAttachmentImage(postID: String) async throws -> Data { guard let url = URL(string: "attachment.php?postid=\(postID)", relativeTo: baseURL) else { throw Error.invalidBaseURL @@ -1062,6 +1098,18 @@ public final class ForumsClient { return try Form(htmlForm, url: url) } + /** + Finds attachment information for a post being edited. + + This method parses the edit form to find any existing attachment + associated with a post, returning the attachment ID and filename. + + - Parameter post: The post to check for attachment information. + + - Returns: A tuple containing the attachment ID and filename, or `nil` if no attachment exists. + + - Throws: An error if the request fails. + */ public func findAttachmentInfo(for post: Post) async throws -> (id: String, filename: String)? { let (data, response) = try await fetch(method: .get, urlString: "editpost.php", parameters: [ "action": "editpost", From 6c03a9681b58a7fd396829169a2c3d718daa2b0c Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:36:50 +1100 Subject: [PATCH 07/13] Another refactor pass --- App/Composition/AttachmentEditView.swift | 4 --- App/Composition/AttachmentPreviewView.swift | 1 + App/Composition/CompositionMenuTree.swift | 34 +++++++++++-------- .../CompositionViewController.swift | 4 ++- App/Posts/ReplyWorkspace.swift | 1 - Awful.xcodeproj/project.pbxproj | 4 --- .../AwfulCore/Model}/ForumAttachment.swift | 33 +++++++++--------- .../AwfulCore/Networking/ForumsClient.swift | 21 ++++++++---- 8 files changed, 54 insertions(+), 48 deletions(-) rename {App/Composition => AwfulCore/Sources/AwfulCore/Model}/ForumAttachment.swift (90%) diff --git a/App/Composition/AttachmentEditView.swift b/App/Composition/AttachmentEditView.swift index 406fdc747..07a7309f1 100644 --- a/App/Composition/AttachmentEditView.swift +++ b/App/Composition/AttachmentEditView.swift @@ -135,8 +135,4 @@ final class AttachmentEditView: UIView { imageView.contentMode = .center } } - - var selectedAction: AttachmentAction { - return actionSegmentedControl.selectedSegmentIndex == 0 ? .keep : .delete - } } diff --git a/App/Composition/AttachmentPreviewView.swift b/App/Composition/AttachmentPreviewView.swift index b8c036b0c..5a6d3c8c6 100644 --- a/App/Composition/AttachmentPreviewView.swift +++ b/App/Composition/AttachmentPreviewView.swift @@ -2,6 +2,7 @@ // // Copyright 2025 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app +import AwfulCore import os import UIKit diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 98941481d..1ed719515 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -2,14 +2,15 @@ // // Copyright 2013 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app +import AwfulCore +import AwfulSettings +import Foil +import ImgurAnonymousAPI import MobileCoreServices import os import Photos import PSMenuItem import UIKit -import AwfulSettings -import Foil -import ImgurAnonymousAPI private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "CompositionMenuTree") @@ -31,6 +32,11 @@ final class CompositionMenuTree: NSObject { private var pendingImage: UIImage? private var pendingImageAssetIdentifier: String? + private func clearPendingImage() { + pendingImage = nil + pendingImageAssetIdentifier = nil + } + /// The textView's class will have some responder chain methods swizzled. init(textView: UITextView, draft: (NSObject & ReplyDraft)? = nil) { self.textView = textView @@ -89,7 +95,8 @@ final class CompositionMenuTree: NSObject { shouldPopWhenMenuHides = true } - + + // fileprivate to allow access from MenuItem action closures defined at file scope fileprivate func showImagePicker(_ sourceType: UIImagePickerController.SourceType) { let picker = UIImagePickerController() picker.sourceType = sourceType @@ -107,7 +114,9 @@ final class CompositionMenuTree: NSObject { } textView.nearestViewController?.present(picker, animated: true, completion: nil) } - + + // MARK: - Imgur Authentication + private func authenticateWithImgur() { guard let viewController = textView.nearestViewController else { return } showAuthenticationPrompt(in: viewController) @@ -314,8 +323,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont insertImage(image) } - pendingImage = nil - pendingImageAssetIdentifier = nil + clearPendingImage() } fileprivate func useForumAttachmentForPendingImage() { @@ -336,8 +344,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont preferredStyle: .alert ) alert.addAction(UIAlertAction(title: "Cancel", style: .cancel) { [weak self] _ in - self?.pendingImage = nil - self?.pendingImageAssetIdentifier = nil + self?.clearPendingImage() }) alert.addAction(UIAlertAction(title: "Resize & Continue", style: .default) { [weak self] _ in self?.resizeAndAttachPendingImage() @@ -350,15 +357,13 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont alertActions: [.ok()] ) textView.nearestViewController?.present(alert, animated: true) - pendingImage = nil - pendingImageAssetIdentifier = nil + clearPendingImage() } return } draft?.forumAttachment = attachment - pendingImage = nil - pendingImageAssetIdentifier = nil + clearPendingImage() onAttachmentChanged?() } @@ -379,8 +384,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } DispatchQueue.main.async { - self.pendingImage = nil - self.pendingImageAssetIdentifier = nil + self.clearPendingImage() guard let resizedAttachment = resizedAttachment else { // Clear placeholder on failure diff --git a/App/Composition/CompositionViewController.swift b/App/Composition/CompositionViewController.swift index 63880e51b..25c2f0618 100644 --- a/App/Composition/CompositionViewController.swift +++ b/App/Composition/CompositionViewController.swift @@ -2,6 +2,7 @@ // // Copyright 2014 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app +import AwfulCore import AwfulSettings import AwfulTheming import UIKit @@ -14,6 +15,7 @@ final class CompositionViewController: ViewController { static let previewHeight: CGFloat = 84 static let editHeight: CGFloat = 120 static let spacing: CGFloat = 8 + static let animationDuration: TimeInterval = 0.3 } override init(nibName: String?, bundle: Bundle?) { @@ -192,7 +194,7 @@ final class CompositionViewController: ViewController { textViewTopConstraint = _textView.topAnchor.constraint(equalTo: anchor, constant: constant) textViewTopConstraint.isActive = true - UIView.animate(withDuration: 0.3) { + UIView.animate(withDuration: AttachmentViewLayout.animationDuration) { self.view.layoutIfNeeded() } } diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index 6dcd3bd43..89be7de48 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -369,7 +369,6 @@ final class NewReplyDraft: NSObject, ReplyDraft { let thread: AwfulThread var text: NSAttributedString? var forumAttachment: ForumAttachment? - var shouldDeleteAttachment = false init(thread: AwfulThread, text: NSAttributedString? = nil) { self.thread = thread diff --git a/Awful.xcodeproj/project.pbxproj b/Awful.xcodeproj/project.pbxproj index ae3c057b9..5d45e26de 100644 --- a/Awful.xcodeproj/project.pbxproj +++ b/Awful.xcodeproj/project.pbxproj @@ -199,7 +199,6 @@ 2D19BA3929C33303009DD94F /* niggly60.json in Resources */ = {isa = PBXBuildFile; fileRef = 2D19BA3629C33302009DD94F /* niggly60.json */; }; 2D19BA3A29C33303009DD94F /* frogrefresh60.json in Resources */ = {isa = PBXBuildFile; fileRef = 2D19BA3729C33302009DD94F /* frogrefresh60.json */; }; 2D19BA3B29C33303009DD94F /* toot60.json in Resources */ = {isa = PBXBuildFile; fileRef = 2D19BA3829C33302009DD94F /* toot60.json */; }; - 2D21C8822EC1C6730089284B /* ForumAttachment.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D21C8812EC1C6730089284B /* ForumAttachment.swift */; }; 2D265F8C292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D265F8B292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift */; }; 2D265F8F292CB447001336ED /* Lottie in Frameworks */ = {isa = PBXBuildFile; productRef = 2D265F8E292CB447001336ED /* Lottie */; }; 2D327DD627F468CE00D21AB0 /* BookmarkColorPicker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D327DD527F468CE00D21AB0 /* BookmarkColorPicker.swift */; }; @@ -518,7 +517,6 @@ 2D19BA3629C33302009DD94F /* niggly60.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = niggly60.json; sourceTree = ""; }; 2D19BA3729C33302009DD94F /* frogrefresh60.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = frogrefresh60.json; sourceTree = ""; }; 2D19BA3829C33302009DD94F /* toot60.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = toot60.json; sourceTree = ""; }; - 2D21C8812EC1C6730089284B /* ForumAttachment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ForumAttachment.swift; sourceTree = ""; }; 2D265F8B292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GetOutFrogRefreshSpinnerView.swift; sourceTree = ""; }; 2D327DD527F468CE00D21AB0 /* BookmarkColorPicker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkColorPicker.swift; sourceTree = ""; }; 2D921268292F588100B16011 /* platinum-member.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "platinum-member.png"; sourceTree = ""; }; @@ -1105,7 +1103,6 @@ children = ( 2DB4472E2EC1E2DA00F03402 /* AttachmentEditView.swift */, 2DB4472F2EC1E2DA00F03402 /* AttachmentPreviewView.swift */, - 2D21C8812EC1C6730089284B /* ForumAttachment.swift */, 1C2C1F0D1CE16FE200CD27DD /* CloseBBcodeTagCommand.swift */, 1C16FB9F1CB492C600C88BD1 /* ComposeField.swift */, 1C16FBA31CB4A41C00C88BD1 /* ComposeTextView.swift */, @@ -1587,7 +1584,6 @@ 1C16FBBA1CB8961F00C88BD1 /* ImageURLProtocol.swift in Sources */, 1C09BFF21A09F86A007C11F5 /* InAppActionCollectionView.swift in Sources */, 1C48538E1F93BADF0042531A /* HTMLRenderingHelpers.swift in Sources */, - 2D21C8822EC1C6730089284B /* ForumAttachment.swift in Sources */, 1C16FC081CC3F01B00C88BD1 /* RapSheetViewController.swift in Sources */, 1CE55A7A1A1072D900E474A6 /* ForumsTableViewController.swift in Sources */, 1C42A2271FD47AEB00F67BA1 /* Forum+Presentation.swift in Sources */, diff --git a/App/Composition/ForumAttachment.swift b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift similarity index 90% rename from App/Composition/ForumAttachment.swift rename to AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift index 5d64db02e..d639e3b62 100644 --- a/App/Composition/ForumAttachment.swift +++ b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift @@ -34,11 +34,11 @@ private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: Conforms to `NSCoding` to support UIKit state restoration. Photo library assets are stored by identifier and reloaded on restoration. */ -final class ForumAttachment: NSObject, NSCoding { +public final class ForumAttachment: NSObject, NSCoding { - static let maxFileSize = 2_097_152 - static let maxDimension = 4096 - static let supportedExtensions = ["gif", "jpg", "jpeg", "png"] + public static let maxFileSize = 2_097_152 + public static let maxDimension = 4096 + public static let supportedExtensions = ["gif", "jpg", "jpeg", "png"] private struct CompressionSettings { static let defaultQuality: CGFloat = 0.9 @@ -48,25 +48,25 @@ final class ForumAttachment: NSObject, NSCoding { static let minimumDimension = 100 } - let image: UIImage? - let photoAssetIdentifier: String? - private(set) var validationError: ValidationError? + public let image: UIImage? + public let photoAssetIdentifier: String? + public private(set) var validationError: ValidationError? - enum ValidationError: Error { + public enum ValidationError: Error { case fileTooLarge(actualSize: Int, maxSize: Int) case dimensionsTooLarge(width: Int, height: Int, maxDimension: Int) case unsupportedFormat case imageDataConversionFailed } - init(image: UIImage, photoAssetIdentifier: String? = nil) { + public init(image: UIImage, photoAssetIdentifier: String? = nil) { self.image = image self.photoAssetIdentifier = photoAssetIdentifier super.init() self.validationError = performValidation() } - required init?(coder: NSCoder) { + public required init?(coder: NSCoder) { if let photoAssetIdentifier = coder.decodeObject(of: NSString.self, forKey: CodingKeys.assetIdentifier.rawValue) { self.photoAssetIdentifier = photoAssetIdentifier as String @@ -105,7 +105,7 @@ final class ForumAttachment: NSObject, NSCoding { self.validationError = performValidation() } - func encode(with coder: NSCoder) { + public func encode(with coder: NSCoder) { if let photoAssetIdentifier = photoAssetIdentifier { coder.encode(photoAssetIdentifier as NSString, forKey: CodingKeys.assetIdentifier.rawValue) } else if let image = image, let imageData = image.pngData() { @@ -118,11 +118,11 @@ final class ForumAttachment: NSObject, NSCoding { case imageData } - var isValid: Bool { + public var isValid: Bool { return validationError == nil } - func validate(maxFileSize: Int? = nil, maxDimension: Int? = nil) -> ValidationError? { + public func validate(maxFileSize: Int? = nil, maxDimension: Int? = nil) -> ValidationError? { guard let image = image else { return .imageDataConversionFailed } @@ -143,6 +143,7 @@ final class ForumAttachment: NSObject, NSCoding { return .fileTooLarge(actualSize: data.count, maxSize: effectiveMaxFileSize) } } catch { + logger.error("Failed to convert image to data during validation: \(error)") return .imageDataConversionFailed } @@ -153,7 +154,7 @@ final class ForumAttachment: NSObject, NSCoding { return validate() } - func imageData() throws -> (data: Data, filename: String, mimeType: String) { + public func imageData() throws -> (data: Data, filename: String, mimeType: String) { guard let image = image else { throw ValidationError.imageDataConversionFailed } @@ -172,7 +173,7 @@ final class ForumAttachment: NSObject, NSCoding { } } - func resized(maxDimension: Int? = nil, maxFileSize: Int? = nil) -> ForumAttachment? { + public func resized(maxDimension: Int? = nil, maxFileSize: Int? = nil) -> ForumAttachment? { guard let originalImage = image else { return nil } let effectiveMaxDimension = maxDimension ?? Self.maxDimension @@ -245,7 +246,7 @@ final class ForumAttachment: NSObject, NSCoding { } extension ForumAttachment.ValidationError { - var localizedDescription: String { + public var localizedDescription: String { switch self { case .fileTooLarge(let actualSize, let maxSize): let formatter = ByteCountFormatter() diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index 5678c85cb..9bcf753d2 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -910,16 +910,17 @@ public final class ForumsClient { // Fallback to SA default attachment limits (2 MB file size, 4096px max dimension) // These values match ForumAttachment.maxFileSize and ForumAttachment.maxDimension - return (maxFileSize: 2_097_152, maxDimension: 4096) + return (maxFileSize: ForumAttachment.maxFileSize, maxDimension: ForumAttachment.maxDimension) } private func parseAttachmentLimits(from document: HTMLDocument) -> (maxFileSize: Int, maxDimension: Int)? { guard let maxFileSizeString = document.firstNode(matchingSelector: "input[name='MAX_FILE_SIZE']")?["value"], let maxFileSize = Int(maxFileSizeString) else { + logger.debug("Could not parse MAX_FILE_SIZE from HTML, falling back to defaults") return nil } - var maxDimension = 4096 + var maxDimension = ForumAttachment.maxDimension for td in document.nodes(matchingSelector: "td") { let text = td.textContent if text.contains("Attach file:") { @@ -936,11 +937,17 @@ public final class ForumsClient { if let nextSibling = nextSibling { let limitsText = nextSibling.textContent let pattern = #"dimensions:\s*(\d+)x(\d+)"# - if let regex = try? NSRegularExpression(pattern: pattern, options: [.caseInsensitive]), - let match = regex.firstMatch(in: limitsText, range: NSRange(limitsText.startIndex..., in: limitsText)), - let dimensionRange = Range(match.range(at: 1), in: limitsText), - let parsedDimension = Int(limitsText[dimensionRange]) { - maxDimension = parsedDimension + do { + let regex = try NSRegularExpression(pattern: pattern, options: [.caseInsensitive]) + if let match = regex.firstMatch(in: limitsText, range: NSRange(limitsText.startIndex..., in: limitsText)), + let dimensionRange = Range(match.range(at: 1), in: limitsText), + let parsedDimension = Int(limitsText[dimensionRange]) { + maxDimension = parsedDimension + } else { + logger.debug("Could not parse dimension limits from text: '\(limitsText)', using default") + } + } catch { + logger.error("Failed to create regex for dimension parsing: \(error)") } } break From 82e9fd9111952fe4823f19cdcf42a14df2a586c6 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:42:36 +1100 Subject: [PATCH 08/13] "Truly this is the final refactor pass" --- App/Resources/RenderView.js | 11 ++--- .../AwfulCore/Model/ForumAttachment.swift | 17 ++++---- .../AwfulCore/Networking/ForumsClient.swift | 40 +------------------ 3 files changed, 15 insertions(+), 53 deletions(-) diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 533499048..a7c25993a 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -105,16 +105,11 @@ Awful.embedTweets = function() { const player = post.target.querySelectorAll("lottie-player"); player.forEach((lottiePlayer) => { lottiePlayer.play(); - // comment out when not testing - //console.log("Lottie playing."); }); } else { - // pause all lottie players if this post is not intersecting const player = post.target.querySelectorAll("lottie-player"); player.forEach((lottiePlayer) => { lottiePlayer.pause(); - // this log is to confirm that pausing actually occurs while scrolling. comment out when not testing - //console.log("Lottie paused."); }); } }); @@ -970,12 +965,14 @@ Awful.loadAttachmentImages = function() { Awful.didFetchAttachmentImage = function(id, dataURL) { var img = document.querySelector('img[data-awful-attachment-id="' + id + '"]'); - if (img) { + if (img && dataURL) { img.src = dataURL; } }; -Awful.loadAttachmentImages(); +if (document.querySelectorAll('img[data-awful-attachment-postid]').length > 0) { + Awful.loadAttachmentImages(); +} // THIS SHOULD STAY AT THE BOTTOM OF THE FILE! // All done; tell the native side we're ready. diff --git a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift index d639e3b62..92c820538 100644 --- a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift +++ b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift @@ -41,10 +41,15 @@ public final class ForumAttachment: NSObject, NSCoding { public static let supportedExtensions = ["gif", "jpg", "jpeg", "png"] private struct CompressionSettings { + /// Initial JPEG compression quality (0.9 provides good balance between file size and visual quality) static let defaultQuality: CGFloat = 0.9 + /// Amount to reduce quality on each compression iteration (0.1 provides smooth quality degradation) static let qualityDecrement: CGFloat = 0.1 + /// Minimum acceptable JPEG quality before switching to dimension reduction (0.1 prevents over-compression artifacts) static let minQuality: CGFloat = 0.1 + /// Factor to scale down dimensions for PNG images with alpha (0.9 provides gradual size reduction) static let dimensionScaleFactor: CGFloat = 0.9 + /// Minimum image dimension to maintain readability (100px ensures text/details remain visible) static let minimumDimension = 100 } @@ -63,7 +68,7 @@ public final class ForumAttachment: NSObject, NSCoding { self.image = image self.photoAssetIdentifier = photoAssetIdentifier super.init() - self.validationError = performValidation() + self.validationError = validate() } public required init?(coder: NSCoder) { @@ -86,12 +91,12 @@ public final class ForumAttachment: NSObject, NSCoding { if let error = requestError { logger.error("Failed to load image from photo library asset: \(error.localizedDescription)") } else if resultImage == nil { - logger.warning("Photo library request returned nil image for asset: \(photoAssetIdentifier as String)") + logger.error("Photo library request returned nil image for asset: \(photoAssetIdentifier as String)") } self.image = resultImage } else { - logger.warning("Photo asset not found for identifier: \(photoAssetIdentifier as String)") + logger.error("Photo asset not found for identifier: \(photoAssetIdentifier as String)") self.image = nil } } else if let imageData = coder.decodeObject(of: NSData.self, forKey: CodingKeys.imageData.rawValue) { @@ -102,7 +107,7 @@ public final class ForumAttachment: NSObject, NSCoding { } super.init() - self.validationError = performValidation() + self.validationError = validate() } public func encode(with coder: NSCoder) { @@ -150,10 +155,6 @@ public final class ForumAttachment: NSObject, NSCoding { return nil } - private func performValidation() -> ValidationError? { - return validate() - } - public func imageData() throws -> (data: Data, filename: String, mimeType: String) { guard let image = image else { throw ValidationError.imageDataConversionFailed diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index 9bcf753d2..0caf2e13c 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -921,7 +921,8 @@ public final class ForumsClient { } var maxDimension = ForumAttachment.maxDimension - for td in document.nodes(matchingSelector: "td") { + // Search only within form table cells to avoid processing unrelated page content + for td in document.nodes(matchingSelector: "form[name='vbform'] td") { let text = td.textContent if text.contains("Attach file:") { var nextSibling: HTMLElement? @@ -957,43 +958,6 @@ public final class ForumsClient { return (maxFileSize: maxFileSize, maxDimension: maxDimension) } - /** - Fetches an attachment image for a post with authentication. - - This method downloads attachment images that require authentication, - typically for posts being edited that already have attachments. - - - Parameter postID: The ID of the post containing the attachment. - - - Returns: The raw image data. - - - Throws: An error if the request fails or returns a non-200 status code. - */ - public func fetchAttachmentImage(postID: String) async throws -> Data { - guard let url = URL(string: "attachment.php?postid=\(postID)", relativeTo: baseURL) else { - throw Error.invalidBaseURL - } - - var request = URLRequest(url: url) - request.httpMethod = "GET" - - guard let session = urlSession else { - throw Error.invalidBaseURL - } - - let (data, response) = try await session.data(for: request) - - guard let httpResponse = response as? HTTPURLResponse else { - throw URLError(.badServerResponse) - } - - guard httpResponse.statusCode == 200 else { - throw URLError(.badServerResponse) - } - - return data - } - public func previewReply( to thread: AwfulThread, bbcode: String From 069bff5039ff93c6946954b1a5b56c296a80ac6d Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:54:08 +1100 Subject: [PATCH 09/13] Another final refactor pass --- App/Composition/AttachmentCardView.swift | 68 ++++++++++++++++ App/Composition/AttachmentEditView.swift | 77 ++++++------------- App/Composition/AttachmentPreviewView.swift | 67 +++++----------- App/Composition/CompositionMenuTree.swift | 9 ++- App/Posts/ReplyWorkspace.swift | 40 +++++++--- App/Resources/Localizable.xcstrings | 72 +++++++++++++++++ Awful.xcodeproj/project.pbxproj | 4 + .../AwfulCore/Networking/ForumsClient.swift | 39 ++++++++++ 8 files changed, 265 insertions(+), 111 deletions(-) create mode 100644 App/Composition/AttachmentCardView.swift diff --git a/App/Composition/AttachmentCardView.swift b/App/Composition/AttachmentCardView.swift new file mode 100644 index 000000000..ce48bf0f1 --- /dev/null +++ b/App/Composition/AttachmentCardView.swift @@ -0,0 +1,68 @@ +// AttachmentCardView.swift +// +// Copyright 2025 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app + +import UIKit + +/// Shared layout constants for attachment card views +enum AttachmentCardLayout { + static let imageSize: CGFloat = 60 + static let imageCornerRadius: CGFloat = 4 + static let cardCornerRadius: CGFloat = 8 + static let cardPadding: CGFloat = 12 + static let imageSpacing: CGFloat = 12 + static let labelTopPadding: CGFloat = 16 + static let titleDetailSpacing: CGFloat = 4 + static let actionButtonSize: CGFloat = 30 +} + +/// Protocol defining common behavior for attachment card views +protocol AttachmentCardView: UIView { + var imageView: UIImageView { get } + var titleLabel: UILabel { get } + var detailLabel: UILabel { get } + + func updateTextColor(_ color: UIColor?) +} + +extension AttachmentCardView { + /// Default implementation for updating text colors + func updateTextColor(_ color: UIColor?) { + titleLabel.textColor = color + detailLabel.textColor = color?.withAlphaComponent(0.7) + } + + /// Creates a standard image view for attachment cards + static func createImageView() -> UIImageView { + let iv = UIImageView() + iv.clipsToBounds = true + iv.layer.cornerRadius = AttachmentCardLayout.imageCornerRadius + iv.backgroundColor = .secondarySystemFill + iv.translatesAutoresizingMaskIntoConstraints = false + return iv + } + + /// Creates a standard title label for attachment cards + static func createTitleLabel(text: String) -> UILabel { + let label = UILabel() + label.font = UIFont.preferredFont(forTextStyle: .subheadline) + label.text = text + label.translatesAutoresizingMaskIntoConstraints = false + return label + } + + /// Creates a standard detail label for attachment cards + static func createDetailLabel() -> UILabel { + let label = UILabel() + label.font = UIFont.preferredFont(forTextStyle: .caption1) + label.textColor = .secondaryLabel + label.translatesAutoresizingMaskIntoConstraints = false + return label + } + + /// Configures the card's appearance with standard styling + func configureCardAppearance() { + backgroundColor = .clear + layer.cornerRadius = AttachmentCardLayout.cardCornerRadius + } +} diff --git a/App/Composition/AttachmentEditView.swift b/App/Composition/AttachmentEditView.swift index 07a7309f1..6bf643086 100644 --- a/App/Composition/AttachmentEditView.swift +++ b/App/Composition/AttachmentEditView.swift @@ -5,36 +5,23 @@ import UIKit /// A card-style view that shows existing attachment info with options to keep or delete it. -final class AttachmentEditView: UIView { +final class AttachmentEditView: UIView, AttachmentCardView { - private let imageView: UIImageView = { - let iv = UIImageView() + let imageView: UIImageView = { + let iv = AttachmentEditView.createImageView() iv.contentMode = .scaleAspectFit - iv.clipsToBounds = true - iv.layer.cornerRadius = 4 - iv.backgroundColor = .secondarySystemFill - iv.translatesAutoresizingMaskIntoConstraints = false return iv }() - private let titleLabel: UILabel = { - let label = UILabel() - label.font = UIFont.preferredFont(forTextStyle: .subheadline) - label.text = "Current Attachment" - label.translatesAutoresizingMaskIntoConstraints = false - return label - }() + let titleLabel: UILabel = AttachmentEditView.createTitleLabel(text: LocalizedString("compose.attachment.edit-title")) - private let detailLabel: UILabel = { - let label = UILabel() - label.font = UIFont.preferredFont(forTextStyle: .caption1) - label.textColor = .secondaryLabel - label.translatesAutoresizingMaskIntoConstraints = false - return label - }() + let detailLabel: UILabel = AttachmentEditView.createDetailLabel() private let actionSegmentedControl: UISegmentedControl = { - let items = ["Keep", "Delete"] + let items = [ + LocalizedString("compose.attachment.action-keep"), + LocalizedString("compose.attachment.action-delete") + ] let sc = UISegmentedControl(items: items) sc.selectedSegmentIndex = 0 sc.translatesAutoresizingMaskIntoConstraints = false @@ -43,16 +30,9 @@ final class AttachmentEditView: UIView { var onActionChanged: ((AttachmentAction) -> Void)? - func updateTextColor(_ color: UIColor?) { - titleLabel.textColor = color - detailLabel.textColor = color?.withAlphaComponent(0.7) - } - func updateSegmentedControlColors(selectedColor: UIColor?) { - // Set normal state text color to white actionSegmentedControl.setTitleTextAttributes([.foregroundColor: UIColor.white], for: .normal) - // Set selected state text color to white with the selected background color if let selectedColor = selectedColor { actionSegmentedControl.setTitleTextAttributes([.foregroundColor: UIColor.white], for: .selected) actionSegmentedControl.selectedSegmentTintColor = selectedColor @@ -74,8 +54,7 @@ final class AttachmentEditView: UIView { } private func setupViews() { - // Background color will be set by the parent view controller based on theme - layer.cornerRadius = 8 + configureCardAppearance() addSubview(imageView) addSubview(titleLabel) @@ -85,27 +64,23 @@ final class AttachmentEditView: UIView { actionSegmentedControl.addTarget(self, action: #selector(actionChanged), for: .valueChanged) NSLayoutConstraint.activate([ - // Image view on the left - imageView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 12), - imageView.topAnchor.constraint(equalTo: topAnchor, constant: 12), - imageView.widthAnchor.constraint(equalToConstant: 60), - imageView.heightAnchor.constraint(equalToConstant: 60), - - // Title label - titleLabel.leadingAnchor.constraint(equalTo: imageView.trailingAnchor, constant: 12), - titleLabel.topAnchor.constraint(equalTo: topAnchor, constant: 16), - titleLabel.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -12), - - // Detail label below title + imageView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: AttachmentCardLayout.cardPadding), + imageView.topAnchor.constraint(equalTo: topAnchor, constant: AttachmentCardLayout.cardPadding), + imageView.widthAnchor.constraint(equalToConstant: AttachmentCardLayout.imageSize), + imageView.heightAnchor.constraint(equalToConstant: AttachmentCardLayout.imageSize), + + titleLabel.leadingAnchor.constraint(equalTo: imageView.trailingAnchor, constant: AttachmentCardLayout.imageSpacing), + titleLabel.topAnchor.constraint(equalTo: topAnchor, constant: AttachmentCardLayout.labelTopPadding), + titleLabel.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -AttachmentCardLayout.cardPadding), + detailLabel.leadingAnchor.constraint(equalTo: titleLabel.leadingAnchor), - detailLabel.topAnchor.constraint(equalTo: titleLabel.bottomAnchor, constant: 4), + detailLabel.topAnchor.constraint(equalTo: titleLabel.bottomAnchor, constant: AttachmentCardLayout.titleDetailSpacing), detailLabel.trailingAnchor.constraint(equalTo: titleLabel.trailingAnchor), - // Segmented control at the bottom - actionSegmentedControl.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 12), - actionSegmentedControl.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -12), - actionSegmentedControl.topAnchor.constraint(equalTo: imageView.bottomAnchor, constant: 12), - actionSegmentedControl.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -12) + actionSegmentedControl.leadingAnchor.constraint(equalTo: leadingAnchor, constant: AttachmentCardLayout.cardPadding), + actionSegmentedControl.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -AttachmentCardLayout.cardPadding), + actionSegmentedControl.topAnchor.constraint(equalTo: imageView.bottomAnchor, constant: AttachmentCardLayout.imageSpacing), + actionSegmentedControl.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -AttachmentCardLayout.cardPadding) ]) } @@ -115,7 +90,7 @@ final class AttachmentEditView: UIView { } func configure(filename: String, filesize: String?, image: UIImage? = nil) { - titleLabel.text = "Current Attachment" + titleLabel.text = LocalizedString("compose.attachment.edit-title") if let filesize = filesize { detailLabel.text = "\(filename) • \(filesize)" } else { @@ -123,12 +98,10 @@ final class AttachmentEditView: UIView { } if let image = image { - // Display the actual attachment image imageView.image = image imageView.tintColor = nil imageView.contentMode = .scaleAspectFit } else { - // Show a generic document icon as fallback let config = UIImage.SymbolConfiguration(pointSize: 40, weight: .light) imageView.image = UIImage(systemName: "doc.fill", withConfiguration: config) imageView.tintColor = .tertiaryLabel diff --git a/App/Composition/AttachmentPreviewView.swift b/App/Composition/AttachmentPreviewView.swift index 5a6d3c8c6..5803b5f22 100644 --- a/App/Composition/AttachmentPreviewView.swift +++ b/App/Composition/AttachmentPreviewView.swift @@ -9,32 +9,17 @@ import UIKit private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "AttachmentPreviewView") /// A card-style view that shows a preview of an attached image with options to remove it. -final class AttachmentPreviewView: UIView { +final class AttachmentPreviewView: UIView, AttachmentCardView { - private let imageView: UIImageView = { - let iv = UIImageView() + let imageView: UIImageView = { + let iv = AttachmentPreviewView.createImageView() iv.contentMode = .scaleAspectFill - iv.clipsToBounds = true - iv.layer.cornerRadius = 4 - iv.translatesAutoresizingMaskIntoConstraints = false return iv }() - private let titleLabel: UILabel = { - let label = UILabel() - label.font = UIFont.preferredFont(forTextStyle: .subheadline) - label.text = "Attachment" - label.translatesAutoresizingMaskIntoConstraints = false - return label - }() + let titleLabel: UILabel = AttachmentPreviewView.createTitleLabel(text: LocalizedString("compose.attachment.preview-title")) - private let detailLabel: UILabel = { - let label = UILabel() - label.font = UIFont.preferredFont(forTextStyle: .caption1) - label.textColor = .secondaryLabel - label.translatesAutoresizingMaskIntoConstraints = false - return label - }() + let detailLabel: UILabel = AttachmentPreviewView.createDetailLabel() private let removeButton: UIButton = { let button = UIButton(type: .system) @@ -47,14 +32,9 @@ final class AttachmentPreviewView: UIView { var onRemove: (() -> Void)? - func updateTextColor(_ color: UIColor?) { - titleLabel.textColor = color - detailLabel.textColor = color?.withAlphaComponent(0.7) - } - func showResizingPlaceholder() { - titleLabel.text = "Resizing Image..." - detailLabel.text = "Please wait" + titleLabel.text = LocalizedString("compose.attachment.resizing-title") + detailLabel.text = LocalizedString("compose.attachment.resizing-message") imageView.image = nil imageView.backgroundColor = .secondarySystemFill } @@ -69,8 +49,7 @@ final class AttachmentPreviewView: UIView { } private func setupViews() { - // Background color will be set by the parent view controller based on theme - layer.cornerRadius = 8 + configureCardAppearance() addSubview(imageView) addSubview(titleLabel) @@ -80,28 +59,24 @@ final class AttachmentPreviewView: UIView { removeButton.addTarget(self, action: #selector(didTapRemove), for: .touchUpInside) NSLayoutConstraint.activate([ - // Image view on the left - imageView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 12), - imageView.topAnchor.constraint(equalTo: topAnchor, constant: 12), - imageView.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -12), - imageView.widthAnchor.constraint(equalToConstant: 60), - imageView.heightAnchor.constraint(equalToConstant: 60), - - // Title label - titleLabel.leadingAnchor.constraint(equalTo: imageView.trailingAnchor, constant: 12), - titleLabel.topAnchor.constraint(equalTo: topAnchor, constant: 16), + imageView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: AttachmentCardLayout.cardPadding), + imageView.topAnchor.constraint(equalTo: topAnchor, constant: AttachmentCardLayout.cardPadding), + imageView.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -AttachmentCardLayout.cardPadding), + imageView.widthAnchor.constraint(equalToConstant: AttachmentCardLayout.imageSize), + imageView.heightAnchor.constraint(equalToConstant: AttachmentCardLayout.imageSize), + + titleLabel.leadingAnchor.constraint(equalTo: imageView.trailingAnchor, constant: AttachmentCardLayout.imageSpacing), + titleLabel.topAnchor.constraint(equalTo: topAnchor, constant: AttachmentCardLayout.labelTopPadding), titleLabel.trailingAnchor.constraint(equalTo: removeButton.leadingAnchor, constant: -8), - // Detail label below title detailLabel.leadingAnchor.constraint(equalTo: titleLabel.leadingAnchor), - detailLabel.topAnchor.constraint(equalTo: titleLabel.bottomAnchor, constant: 4), + detailLabel.topAnchor.constraint(equalTo: titleLabel.bottomAnchor, constant: AttachmentCardLayout.titleDetailSpacing), detailLabel.trailingAnchor.constraint(equalTo: titleLabel.trailingAnchor), - // Remove button on the right - removeButton.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -12), + removeButton.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -AttachmentCardLayout.cardPadding), removeButton.centerYAnchor.constraint(equalTo: centerYAnchor), - removeButton.widthAnchor.constraint(equalToConstant: 30), - removeButton.heightAnchor.constraint(equalToConstant: 30) + removeButton.widthAnchor.constraint(equalToConstant: AttachmentCardLayout.actionButtonSize), + removeButton.heightAnchor.constraint(equalToConstant: AttachmentCardLayout.actionButtonSize) ]) } @@ -110,7 +85,7 @@ final class AttachmentPreviewView: UIView { } func configure(with attachment: ForumAttachment) { - titleLabel.text = "Attachment" + titleLabel.text = LocalizedString("compose.attachment.preview-title") imageView.backgroundColor = .clear imageView.image = attachment.image diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 1ed719515..505c467e3 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -16,7 +16,7 @@ private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: /// Can take over UIMenuController to show a tree of composition-related items on behalf of a text view. final class CompositionMenuTree: NSObject { - // This class exists to expose the struct-defined menu to Objective-C and to act as an image picker delegate. + // Manages composition menu items and acts as image picker delegate for attachment handling. @FoilDefaultStorage(Settings.imgurUploadMode) private var imgurUploadMode @@ -31,10 +31,12 @@ final class CompositionMenuTree: NSObject { private var pendingImage: UIImage? private var pendingImageAssetIdentifier: String? + private var isProcessingImage = false private func clearPendingImage() { pendingImage = nil pendingImageAssetIdentifier = nil + isProcessingImage = false } /// The textView's class will have some responder chain methods swizzled. @@ -328,7 +330,12 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont fileprivate func useForumAttachmentForPendingImage() { guard let image = pendingImage else { return } + guard !isProcessingImage else { + logger.warning("Image already being processed, ignoring concurrent request") + return + } + isProcessingImage = true let attachment = ForumAttachment(image: image, photoAssetIdentifier: pendingImageAssetIdentifier) if let error = attachment.validationError { // Check if the error can be fixed by resizing diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index 89be7de48..2a65c8548 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -495,12 +495,23 @@ final class EditReplyDraft: NSObject, ReplyDraft { } extension NewReplyDraft: SubmittableDraft { + enum SubmissionError: LocalizedError { + case emptyText + case attachmentValidationFailed(ForumAttachment.ValidationError) + + var errorDescription: String? { + switch self { + case .emptyText: + return "Post text cannot be empty" + case .attachmentValidationFailed(let validationError): + return validationError.localizedDescription + } + } + } + func submit(_ completion: @escaping (Error?) -> Void) -> Progress { guard let text = text else { - let error = NSError(domain: "Awful", code: 0, userInfo: [ - NSLocalizedDescriptionKey: "Post text cannot be empty" - ]) - completion(error) + completion(SubmissionError.emptyText) return Progress(totalUnitCount: 1) } @@ -518,10 +529,7 @@ extension NewReplyDraft: SubmittableDraft { maxFileSize: limits.maxFileSize, maxDimension: limits.maxDimension ) { - let error = NSError(domain: "Awful", code: 0, userInfo: [ - NSLocalizedDescriptionKey: validationError.localizedDescription - ]) - completion(error) + completion(SubmissionError.attachmentValidationFailed(validationError)) return } attachmentData = try forumAttachment.imageData() @@ -539,12 +547,20 @@ extension NewReplyDraft: SubmittableDraft { } extension EditReplyDraft: SubmittableDraft { + enum SubmissionError: LocalizedError { + case emptyText + + var errorDescription: String? { + switch self { + case .emptyText: + return "Post text cannot be empty" + } + } + } + func submit(_ completion: @escaping (Error?) -> Void) -> Progress { guard let text = text else { - let error = NSError(domain: "Awful", code: 0, userInfo: [ - NSLocalizedDescriptionKey: "Post text cannot be empty" - ]) - completion(error) + completion(SubmissionError.emptyText) return Progress(totalUnitCount: 1) } diff --git a/App/Resources/Localizable.xcstrings b/App/Resources/Localizable.xcstrings index 786d05083..4ee76999b 100644 --- a/App/Resources/Localizable.xcstrings +++ b/App/Resources/Localizable.xcstrings @@ -90,6 +90,78 @@ } } }, + "compose.attachment.action-delete" : { + "comment" : "Segmented control option to delete an existing attachment.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Delete" + } + } + } + }, + "compose.attachment.action-keep" : { + "comment" : "Segmented control option to keep an existing attachment.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Keep" + } + } + } + }, + "compose.attachment.edit-title" : { + "comment" : "Title shown on the attachment edit card.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Current Attachment" + } + } + } + }, + "compose.attachment.preview-title" : { + "comment" : "Title shown on the attachment preview card.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Attachment" + } + } + } + }, + "compose.attachment.resizing-message" : { + "comment" : "Message shown while an attachment is being resized.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Please wait" + } + } + } + }, + "compose.attachment.resizing-title" : { + "comment" : "Title shown while an attachment is being resized.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Resizing Image..." + } + } + } + }, "compose.cancel-menu.delete-draft" : { "comment" : "Destructive button in action sheet that deletes the draft.", "localizations" : { diff --git a/Awful.xcodeproj/project.pbxproj b/Awful.xcodeproj/project.pbxproj index 5d45e26de..d2c1f643a 100644 --- a/Awful.xcodeproj/project.pbxproj +++ b/Awful.xcodeproj/project.pbxproj @@ -202,6 +202,7 @@ 2D265F8C292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D265F8B292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift */; }; 2D265F8F292CB447001336ED /* Lottie in Frameworks */ = {isa = PBXBuildFile; productRef = 2D265F8E292CB447001336ED /* Lottie */; }; 2D327DD627F468CE00D21AB0 /* BookmarkColorPicker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D327DD527F468CE00D21AB0 /* BookmarkColorPicker.swift */; }; + 2D571B472EC83DD00026826C /* AttachmentCardView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D571B462EC83DD00026826C /* AttachmentCardView.swift */; }; 2D921269292F588100B16011 /* platinum-member.png in Resources */ = {isa = PBXBuildFile; fileRef = 2D921268292F588100B16011 /* platinum-member.png */; }; 2DAF1FE12E05D3ED006F6BC4 /* View+FontDesign.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DAF1FE02E05D3EB006F6BC4 /* View+FontDesign.swift */; }; 2DB447302EC1E2DA00F03402 /* AttachmentPreviewView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DB4472F2EC1E2DA00F03402 /* AttachmentPreviewView.swift */; }; @@ -519,6 +520,7 @@ 2D19BA3829C33302009DD94F /* toot60.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = toot60.json; sourceTree = ""; }; 2D265F8B292CB429001336ED /* GetOutFrogRefreshSpinnerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GetOutFrogRefreshSpinnerView.swift; sourceTree = ""; }; 2D327DD527F468CE00D21AB0 /* BookmarkColorPicker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkColorPicker.swift; sourceTree = ""; }; + 2D571B462EC83DD00026826C /* AttachmentCardView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttachmentCardView.swift; sourceTree = ""; }; 2D921268292F588100B16011 /* platinum-member.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "platinum-member.png"; sourceTree = ""; }; 2DAF1FE02E05D3EB006F6BC4 /* View+FontDesign.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "View+FontDesign.swift"; sourceTree = ""; }; 2DB4472E2EC1E2DA00F03402 /* AttachmentEditView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttachmentEditView.swift; sourceTree = ""; }; @@ -1101,6 +1103,7 @@ 1CF2462016DD957300C75E05 /* Composition */ = { isa = PBXGroup; children = ( + 2D571B462EC83DD00026826C /* AttachmentCardView.swift */, 2DB4472E2EC1E2DA00F03402 /* AttachmentEditView.swift */, 2DB4472F2EC1E2DA00F03402 /* AttachmentPreviewView.swift */, 1C2C1F0D1CE16FE200CD27DD /* CloseBBcodeTagCommand.swift */, @@ -1613,6 +1616,7 @@ 1C4506C41A2BAB3800767306 /* Handoff.swift in Sources */, 1C16FBFE1CBF237800C88BD1 /* PrivateMessageInboxRefresher.swift in Sources */, 1C3E181B224FAE1200BD88E5 /* SmilieDataStore+Shared.swift in Sources */, + 2D571B472EC83DD00026826C /* AttachmentCardView.swift in Sources */, 1C16FC1A1CD42EB300C88BD1 /* PostPreviewViewController.swift in Sources */, 1C24BC962002BF2F0022C85F /* ForumListCell.swift in Sources */, 1C4EAD5E1BC0622D0008BE54 /* AwfulCore.swift in Sources */, diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index 0caf2e13c..2e21e93d8 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -1104,6 +1104,45 @@ public final class ForumsClient { return (id: attachmentID, filename: filename) } + /** + Fetches the image data for an attachment associated with a post. + + - Parameter postID: The ID of the post containing the attachment. + - Returns: The image data for the attachment. + - Throws: An error if the request fails or no attachment is found. + */ + public func fetchAttachmentImage(postID: String) async throws -> Data { + // First, get the attachment info from the edit page + guard let urlSession else { throw Error.missingURLSession } + + let (data, response) = try await fetch(method: .get, urlString: "editpost.php", parameters: [ + "action": "editpost", + "postid": postID, + ]) + let (document, _) = try parseHTML(data: data, response: response) + + guard let attachmentLink = document.firstNode(matchingSelector: "a[href*='attachment.php']"), + let href = attachmentLink["href"], + let components = URLComponents(string: href), + let queryItems = components.queryItems, + let attachmentID = queryItems.first(where: { $0.name == "attachmentid" })?.value else { + throw NSError(domain: "Awful", code: 0, userInfo: [ + NSLocalizedDescriptionKey: "No attachment found for this post" + ]) + } + + // Now fetch the actual attachment image + guard let attachmentURL = URL(string: "attachment.php?attachmentid=\(attachmentID)", relativeTo: baseURL) else { + throw Error.invalidBaseURL + } + + var request = URLRequest(url: attachmentURL) + request.httpMethod = "GET" + + let (imageData, _) = try await urlSession.data(for: request) + return imageData + } + /** - Parameter postID: The post's ID. Specified directly in case no such post exists, which would make for a useless `Post`. - Returns: The promise of a post (with its `thread` set) and the page containing the post (may be `AwfulThreadPage.last`). From 96acda536a5313c96b0766212e97c22f086913f5 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 16:06:17 +1100 Subject: [PATCH 10/13] Refactoring --- App/Composition/AttachmentCardView.swift | 49 +++-- App/Composition/AttachmentEditView.swift | 22 +- App/Composition/AttachmentPreviewView.swift | 22 +- App/Composition/CompositionMenuTree.swift | 201 +++++++++++------- App/Misc/HTMLRenderingHelpers.swift | 11 +- .../AwfulCore/Networking/ForumsClient.swift | 193 ++++++++++++----- .../URLRequest+MultipartFormData.swift | 33 +++ 7 files changed, 345 insertions(+), 186 deletions(-) diff --git a/App/Composition/AttachmentCardView.swift b/App/Composition/AttachmentCardView.swift index ce48bf0f1..7a72c1bb9 100644 --- a/App/Composition/AttachmentCardView.swift +++ b/App/Composition/AttachmentCardView.swift @@ -16,52 +16,51 @@ enum AttachmentCardLayout { static let actionButtonSize: CGFloat = 30 } -/// Protocol defining common behavior for attachment card views -protocol AttachmentCardView: UIView { - var imageView: UIImageView { get } - var titleLabel: UILabel { get } - var detailLabel: UILabel { get } +/// Base class for attachment card views with common UI elements and styling +class AttachmentCardView: UIView { - func updateTextColor(_ color: UIColor?) -} - -extension AttachmentCardView { - /// Default implementation for updating text colors - func updateTextColor(_ color: UIColor?) { - titleLabel.textColor = color - detailLabel.textColor = color?.withAlphaComponent(0.7) - } - - /// Creates a standard image view for attachment cards - static func createImageView() -> UIImageView { + let imageView: UIImageView = { let iv = UIImageView() iv.clipsToBounds = true iv.layer.cornerRadius = AttachmentCardLayout.imageCornerRadius iv.backgroundColor = .secondarySystemFill iv.translatesAutoresizingMaskIntoConstraints = false return iv - } + }() - /// Creates a standard title label for attachment cards - static func createTitleLabel(text: String) -> UILabel { + let titleLabel: UILabel = { let label = UILabel() label.font = UIFont.preferredFont(forTextStyle: .subheadline) - label.text = text label.translatesAutoresizingMaskIntoConstraints = false return label - } + }() - /// Creates a standard detail label for attachment cards - static func createDetailLabel() -> UILabel { + let detailLabel: UILabel = { let label = UILabel() label.font = UIFont.preferredFont(forTextStyle: .caption1) label.textColor = .secondaryLabel label.translatesAutoresizingMaskIntoConstraints = false return label + }() + + override init(frame: CGRect) { + super.init(frame: frame) + configureCardAppearance() + } + + required init?(coder: NSCoder) { + super.init(coder: coder) + configureCardAppearance() + } + + /// Updates the text color for title and detail labels + func updateTextColor(_ color: UIColor?) { + titleLabel.textColor = color + detailLabel.textColor = color?.withAlphaComponent(0.7) } /// Configures the card's appearance with standard styling - func configureCardAppearance() { + private func configureCardAppearance() { backgroundColor = .clear layer.cornerRadius = AttachmentCardLayout.cardCornerRadius } diff --git a/App/Composition/AttachmentEditView.swift b/App/Composition/AttachmentEditView.swift index 6bf643086..31353d5d5 100644 --- a/App/Composition/AttachmentEditView.swift +++ b/App/Composition/AttachmentEditView.swift @@ -5,17 +5,7 @@ import UIKit /// A card-style view that shows existing attachment info with options to keep or delete it. -final class AttachmentEditView: UIView, AttachmentCardView { - - let imageView: UIImageView = { - let iv = AttachmentEditView.createImageView() - iv.contentMode = .scaleAspectFit - return iv - }() - - let titleLabel: UILabel = AttachmentEditView.createTitleLabel(text: LocalizedString("compose.attachment.edit-title")) - - let detailLabel: UILabel = AttachmentEditView.createDetailLabel() +final class AttachmentEditView: AttachmentCardView { private let actionSegmentedControl: UISegmentedControl = { let items = [ @@ -44,17 +34,19 @@ final class AttachmentEditView: UIView, AttachmentCardView { case delete } - init() { - super.init(frame: .zero) + override init(frame: CGRect) { + super.init(frame: frame) setupViews() } required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") + super.init(coder: coder) + setupViews() } private func setupViews() { - configureCardAppearance() + imageView.contentMode = .scaleAspectFit + titleLabel.text = LocalizedString("compose.attachment.edit-title") addSubview(imageView) addSubview(titleLabel) diff --git a/App/Composition/AttachmentPreviewView.swift b/App/Composition/AttachmentPreviewView.swift index 5803b5f22..ff7904b7d 100644 --- a/App/Composition/AttachmentPreviewView.swift +++ b/App/Composition/AttachmentPreviewView.swift @@ -9,17 +9,7 @@ import UIKit private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "AttachmentPreviewView") /// A card-style view that shows a preview of an attached image with options to remove it. -final class AttachmentPreviewView: UIView, AttachmentCardView { - - let imageView: UIImageView = { - let iv = AttachmentPreviewView.createImageView() - iv.contentMode = .scaleAspectFill - return iv - }() - - let titleLabel: UILabel = AttachmentPreviewView.createTitleLabel(text: LocalizedString("compose.attachment.preview-title")) - - let detailLabel: UILabel = AttachmentPreviewView.createDetailLabel() +final class AttachmentPreviewView: AttachmentCardView { private let removeButton: UIButton = { let button = UIButton(type: .system) @@ -39,17 +29,19 @@ final class AttachmentPreviewView: UIView, AttachmentCardView { imageView.backgroundColor = .secondarySystemFill } - init() { - super.init(frame: .zero) + override init(frame: CGRect) { + super.init(frame: frame) setupViews() } required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") + super.init(coder: coder) + setupViews() } private func setupViews() { - configureCardAppearance() + imageView.contentMode = .scaleAspectFill + titleLabel.text = LocalizedString("compose.attachment.preview-title") addSubview(imageView) addSubview(titleLabel) diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 505c467e3..30c4aaf73 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -29,9 +29,19 @@ final class CompositionMenuTree: NSObject { var onAttachmentChanged: (() -> Void)? var onResizingStarted: (() -> Void)? + private let imageProcessingQueue = DispatchQueue(label: "com.awful.attachment.processing", qos: .userInitiated) private var pendingImage: UIImage? private var pendingImageAssetIdentifier: String? - private var isProcessingImage = false + private var _isProcessingImage = false + + private var isProcessingImage: Bool { + get { + imageProcessingQueue.sync { _isProcessingImage } + } + set { + imageProcessingQueue.sync { _isProcessingImage = newValue } + } + } private func clearPendingImage() { pendingImage = nil @@ -125,23 +135,20 @@ final class CompositionMenuTree: NSObject { } private func showAuthenticationPrompt(in viewController: UIViewController) { - let alert = UIAlertController( + presentAlert( + in: viewController, title: "Imgur Authentication Required", message: "You've enabled Imgur Account uploads in settings. To upload images with your account, you'll need to log in to Imgur.", - preferredStyle: .alert + actions: [ + ("Log In", .default, { [weak self] in + self?.performAuthentication(in: viewController) + }), + ("Use Anonymous Upload", .default, { [weak self] in + self?.switchToAnonymousUploads() + }), + ("Cancel", .cancel, nil) + ] ) - - alert.addAction(UIAlertAction(title: "Log In", style: .default) { [weak self] _ in - self?.performAuthentication(in: viewController) - }) - - alert.addAction(UIAlertAction(title: "Use Anonymous Upload", style: .default) { [weak self] _ in - self?.switchToAnonymousUploads() - }) - - alert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) - - viewController.present(alert, animated: true) } private func performAuthentication(in viewController: UIViewController) { @@ -175,59 +182,72 @@ final class CompositionMenuTree: NSObject { } private func presentAuthenticationSuccessAlert(in viewController: UIViewController) { - let alert = UIAlertController( + presentAlert( + in: viewController, title: "Successfully Logged In", message: "You're now logged in to Imgur and can upload images with your account.", - preferredStyle: .alert + actions: [ + ("Continue", .default, { [weak self] in + self?.showImagePicker(.photoLibrary) + }) + ] ) - - alert.addAction(UIAlertAction(title: "Continue", style: .default) { [weak self] _ in - self?.showImagePicker(.photoLibrary) - }) - - viewController.present(alert, animated: true) } private func presentRateLimitAlert(in viewController: UIViewController) { - let alert = UIAlertController( + presentAlert( + in: viewController, title: "Imgur Rate Limit Exceeded", message: "Imgur's API is currently rate limited. You can try again later or use anonymous uploads for now.", - preferredStyle: .alert + actions: [ + ("Use Anonymous Uploads", .default, { [weak self] in + self?.switchToAnonymousUploads() + }), + ("Cancel", .cancel, nil) + ] ) - - alert.addAction(UIAlertAction(title: "Use Anonymous Uploads", style: .default) { [weak self] _ in - self?.switchToAnonymousUploads() - }) - - alert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) - - viewController.present(alert, animated: true) } private func presentAuthenticationFailureAlert(in viewController: UIViewController) { - let alert = UIAlertController( + presentAlert( + in: viewController, title: "Authentication Failed", message: "Could not log in to Imgur. You can try again or choose anonymous uploads in settings.", - preferredStyle: .alert + actions: [ + ("Try Again", .default, { [weak self] in + self?.authenticateWithImgur() + }), + ("Use Anonymous Upload", .default, { [weak self] in + self?.switchToAnonymousUploads() + }), + ("Cancel", .cancel, nil) + ] ) - - alert.addAction(UIAlertAction(title: "Try Again", style: .default) { [weak self] _ in - self?.authenticateWithImgur() - }) - - alert.addAction(UIAlertAction(title: "Use Anonymous Upload", style: .default) { [weak self] _ in - self?.switchToAnonymousUploads() - }) - - alert.addAction(UIAlertAction(title: "Cancel", style: .cancel)) - - viewController.present(alert, animated: true) } private func switchToAnonymousUploads() { imgurUploadMode = .anonymous showImagePicker(.photoLibrary) } + + // MARK: - Alert Helper + + private func presentAlert( + in viewController: UIViewController, + title: String, + message: String, + actions: [(title: String, style: UIAlertAction.Style, handler: (() -> Void)?)] + ) { + let alert = UIAlertController(title: title, message: message, preferredStyle: .alert) + + for (actionTitle, style, handler) in actions { + alert.addAction(UIAlertAction(title: actionTitle, style: style) { _ in + handler?() + }) + } + + viewController.present(alert, animated: true) + } func insertImage(_ image: UIImage, withAssetIdentifier assetID: String? = nil) { // Inserting the image changes our font and text color, so save those now and restore those later. @@ -345,25 +365,28 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } if canResize { - let alert = UIAlertController( + guard let viewController = textView.nearestViewController else { return } + presentAlert( + in: viewController, title: "Attachment Too Large", message: "\(error.localizedDescription)\n\nWould you like to automatically resize the image to fit?", - preferredStyle: .alert + actions: [ + ("Cancel", .cancel, { [weak self] in + self?.clearPendingImage() + }), + ("Resize & Continue", .default, { [weak self] in + self?.resizeAndAttachPendingImage() + }) + ] ) - alert.addAction(UIAlertAction(title: "Cancel", style: .cancel) { [weak self] _ in - self?.clearPendingImage() - }) - alert.addAction(UIAlertAction(title: "Resize & Continue", style: .default) { [weak self] _ in - self?.resizeAndAttachPendingImage() - }) - textView.nearestViewController?.present(alert, animated: true) } else { - let alert = UIAlertController( + guard let viewController = textView.nearestViewController else { return } + presentAlert( + in: viewController, title: "Invalid Attachment", message: error.localizedDescription, - alertActions: [.ok()] + actions: [("OK", .default, nil)] ) - textView.nearestViewController?.present(alert, animated: true) clearPendingImage() } return @@ -376,49 +399,81 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } private func resizeAndAttachPendingImage() { - guard let image = pendingImage else { return } + guard let image = pendingImage, let assetID = pendingImageAssetIdentifier else { + // Use weak capture to prevent retaining the full image if not needed + guard let image = pendingImage else { return } + resizeAndAttach(image: image, assetIdentifier: nil) + return + } + resizeAndAttach(image: image, assetIdentifier: assetID) + } + private func resizeAndAttach(image: UIImage, assetIdentifier: String?) { // Show resizing placeholder immediately onResizingStarted?() - // Perform resize in background - DispatchQueue.global(qos: .userInitiated).async { [weak self] in + // Perform resize in background with weak image reference to prevent memory leak + imageProcessingQueue.async { [weak self] in guard let self = self else { return } let resizedAttachment = autoreleasepool { () -> ForumAttachment? in - let originalAttachment = ForumAttachment(image: image, photoAssetIdentifier: self.pendingImageAssetIdentifier) + let originalAttachment = ForumAttachment(image: image, photoAssetIdentifier: assetIdentifier) return originalAttachment.resized() } - DispatchQueue.main.async { - self.clearPendingImage() + DispatchQueue.main.async { [weak self] in + guard let self = self else { return } guard let resizedAttachment = resizedAttachment else { + // Don't clear pendingImage - allow retry + self.isProcessingImage = false // Clear placeholder on failure self.onAttachmentChanged?() - let alert = UIAlertController( + guard let viewController = self.textView.nearestViewController else { return } + self.presentAlert( + in: viewController, title: "Resize Failed", - message: "Unable to resize image to meet requirements. Please try a different image.", - alertActions: [.ok()] + message: "Unable to resize image to meet requirements.", + actions: [ + ("Try Again", .default, { [weak self] in + self?.resizeAndAttachPendingImage() + }), + ("Cancel", .cancel, { [weak self] in + self?.clearPendingImage() + self?.onAttachmentChanged?() + }) + ] ) - self.textView.nearestViewController?.present(alert, animated: true) return } if let error = resizedAttachment.validationError { + // Don't clear pendingImage - allow retry + self.isProcessingImage = false // Clear placeholder on validation failure self.onAttachmentChanged?() - let alert = UIAlertController( + guard let viewController = self.textView.nearestViewController else { return } + self.presentAlert( + in: viewController, title: "Resize Failed", - message: "\(error.localizedDescription)\n\nPlease try a different image.", - alertActions: [.ok()] + message: "\(error.localizedDescription)", + actions: [ + ("Try Again", .default, { [weak self] in + self?.resizeAndAttachPendingImage() + }), + ("Cancel", .cancel, { [weak self] in + self?.clearPendingImage() + self?.onAttachmentChanged?() + }) + ] ) - self.textView.nearestViewController?.present(alert, animated: true) return } + // Success - now we can clear the pending image + self.clearPendingImage() // Update with resized attachment self.draft?.forumAttachment = resizedAttachment self.onAttachmentChanged?() diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index 2cff145db..a49e5f946 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -155,7 +155,11 @@ extension HTMLDocument { if src.hasPrefix("http") { attachmentSrc = src } else if let baseURL = ForumsClient.shared.baseURL { - attachmentSrc = baseURL.absoluteString + src + // Use URLComponents for safe URL construction instead of string concatenation + guard let url = URL(string: src, relativeTo: baseURL)?.absoluteString else { + continue + } + attachmentSrc = url } else { continue } @@ -480,8 +484,9 @@ private func randomwaffleURLForWaffleimagesURL(_ url: URL) -> URL? { pathExtension = "jpg" } - // Pretty sure NSURLComponents init should always succeed from a URL. - var components = URLComponents(url: url, resolvingAgainstBaseURL: true)! + guard var components = URLComponents(url: url, resolvingAgainstBaseURL: true) else { + return nil + } components.host = "randomwaffle.gbs.fm" components.path = "/images/\(hashPrefix)/\(hash).\(pathExtension)" return components.url diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index 2e21e93d8..80867faa6 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -816,72 +816,134 @@ public final class ForumsClient { let (wasThreadClosed, threadID) = await thread.managedObjectContext!.perform { (thread.closed, thread.threadID) } - let formParams: [KeyValuePairs.Element] - var parsedLimits: (maxFileSize: Int, maxDimension: Int)? - do { - let (data, response) = try await fetch(method: .get, urlString: "newreply.php", parameters: [ - "action": "newreply", - "threadid": threadID, - ]) - let (document, url) = try parseHTML(data: data, response: response) - parsedLimits = parseAttachmentLimits(from: document) + // Fetch and prepare the reply form + let (formParams, parsedLimits) = try await fetchAndPrepareReplyForm( + threadID: threadID, + bbcode: bbcode, + wasThreadClosed: wasThreadClosed + ) - guard let htmlForm = document.firstNode(matchingSelector: "form[name='vbform']") else { - let description = if wasThreadClosed { - "Could not reply; the thread may be closed." - } else { - "Could not reply; failed to find the form." - } - throw AwfulCoreError.parseError(description: description) - } - let parsedForm = try Form(htmlForm, url: url) - let form = SubmittableForm(parsedForm) - try form.enter(text: bbcode, for: "message") - let submission = form.submit(button: parsedForm.submitButton(named: "submit")) - formParams = prepareFormEntries(submission) - } + // Submit the reply + let postID = try await submitReply( + formParams: formParams, + attachment: attachment + ) - let postID: String? - do { - let (data, response): (Data, URLResponse) - if let attachment = attachment { - guard let urlSession else { - throw Error.missingURLSession - } - guard let url = URL(string: "newreply.php", relativeTo: baseURL) else { - throw Error.invalidBaseURL - } - var request = URLRequest(url: url) - request.httpMethod = "POST" - let escapedParams = formParams.lazy.map(win1252Escaped(_:)) - try request.setMultipartFormData(escapedParams, encoding: .windowsCP1252) - try request.appendFileData(attachment.data, withName: "attachment", filename: attachment.filename, mimeType: attachment.mimeType) - (data, response) = try await urlSession.data(for: request) + // Determine reply location + let location = await determineReplyLocation(postID: postID, in: mainContext) + + return (location: location, attachmentLimits: parsedLimits) + } + + /// Fetches the reply form and prepares form parameters + private func fetchAndPrepareReplyForm( + threadID: String, + bbcode: String, + wasThreadClosed: Bool + ) async throws -> (formParams: [KeyValuePairs.Element], limits: (maxFileSize: Int, maxDimension: Int)?) { + let (data, response) = try await fetch(method: .get, urlString: "newreply.php", parameters: [ + "action": "newreply", + "threadid": threadID, + ]) + let (document, url) = try parseHTML(data: data, response: response) + + let parsedLimits = parseAttachmentLimits(from: document) + + guard let htmlForm = document.firstNode(matchingSelector: "form[name='vbform']") else { + let description = if wasThreadClosed { + "Could not reply; the thread may be closed." } else { - (data, response) = try await fetch(method: .post, urlString: "newreply.php", parameters: formParams) + "Could not reply; failed to find the form." } - let (document, _) = try parseHTML(data: data, response: response) - let link = document.firstNode(matchingSelector: "a[href *= 'goto=post']") + throw AwfulCoreError.parseError(description: description) + } + + let parsedForm = try Form(htmlForm, url: url) + let form = SubmittableForm(parsedForm) + try form.enter(text: bbcode, for: "message") + let submission = form.submit(button: parsedForm.submitButton(named: "submit")) + let formParams = prepareFormEntries(submission) + + return (formParams: formParams, limits: parsedLimits) + } + + /// Submits the reply with or without attachment + private func submitReply( + formParams: [KeyValuePairs.Element], + attachment: (data: Data, filename: String, mimeType: String)? + ) async throws -> String? { + let (data, response): (Data, URLResponse) + + if let attachment = attachment { + (data, response) = try await submitReplyWithAttachment( + formParams: formParams, + attachment: attachment + ) + } else { + (data, response) = try await fetch(method: .post, urlString: "newreply.php", parameters: formParams) + } + + return try parseReplyResponse(data: data, response: response) + } + + /// Submits reply with multipart form data for attachment + private func submitReplyWithAttachment( + formParams: [KeyValuePairs.Element], + attachment: (data: Data, filename: String, mimeType: String) + ) async throws -> (Data, URLResponse) { + guard let urlSession else { + throw Error.missingURLSession + } + guard let url = URL(string: "newreply.php", relativeTo: baseURL) else { + throw Error.invalidBaseURL + } + + var request = URLRequest(url: url) + request.httpMethod = "POST" + let escapedParams = formParams.lazy.map(win1252Escaped(_:)) + try request.setMultipartFormData(escapedParams, encoding: .windowsCP1252) + try request.appendFileData( + attachment.data, + withName: "attachment", + filename: attachment.filename, + mimeType: attachment.mimeType + ) + + return try await urlSession.data(for: request) + } + + /// Parses the reply response to extract post ID + private func parseReplyResponse(data: Data, response: URLResponse) throws -> String? { + let (document, _) = try parseHTML(data: data, response: response) + + let link = document.firstNode(matchingSelector: "a[href *= 'goto=post']") ?? document.firstNode(matchingSelector: "a[href *= 'goto=lastpost']") - let queryItems = link - .flatMap { $0["href"] } - .flatMap { URLComponents(string: $0) } - .flatMap { $0.queryItems } - postID = if let goto = queryItems?.first(where: { $0.name == "goto" }), goto.value == "post" { - queryItems?.first(where: { $0.name == "postid" })?.value - } else { - nil - } + + let queryItems = link + .flatMap { $0["href"] } + .flatMap { URLComponents(string: $0) } + .flatMap { $0.queryItems } + + if let goto = queryItems?.first(where: { $0.name == "goto" }), goto.value == "post" { + return queryItems?.first(where: { $0.name == "postid" })?.value } - let location = await mainContext.perform { + + return nil + } + + /// Determines the reply location from post ID + private func determineReplyLocation( + postID: String?, + in context: NSManagedObjectContext + ) async -> ReplyLocation { + await context.perform { if let postID { - ReplyLocation.post(Post.objectForKey(objectKey: PostKey(postID: postID), in: mainContext)) + ReplyLocation.post(Post.objectForKey(objectKey: PostKey(postID: postID), in: context)) } else { ReplyLocation.lastPostInThread } } - return (location: location, attachmentLimits: parsedLimits) } /** @@ -1093,7 +1155,8 @@ public final class ForumsClient { return nil } - let filename = attachmentLink.textContent.trimmingCharacters(in: .whitespacesAndNewlines) + let rawFilename = attachmentLink.textContent.trimmingCharacters(in: .whitespacesAndNewlines) + let filename = sanitizeFilename(rawFilename) guard !filename.isEmpty, let components = URLComponents(string: href), let queryItems = components.queryItems, @@ -1675,3 +1738,23 @@ private func findIgnoreFormkey(in parsed: ParsedDocument) throws -> String { .map { $0["value"] } ?? "" } + +private func sanitizeFilename(_ filename: String) -> String { + // Remove path traversal attempts and dangerous characters + var sanitized = filename + .replacingOccurrences(of: "../", with: "") + .replacingOccurrences(of: "..\\", with: "") + .replacingOccurrences(of: "/", with: "_") + .replacingOccurrences(of: "\\", with: "_") + .replacingOccurrences(of: "\0", with: "") + + // Remove leading/trailing dots and spaces which can cause issues on some filesystems + sanitized = sanitized.trimmingCharacters(in: CharacterSet(charactersIn: ". ")) + + // Limit length to reasonable maximum (most filesystems support 255 bytes) + if sanitized.utf8.count > 255 { + sanitized = String(sanitized.prefix(255)) + } + + return sanitized +} diff --git a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift index 17419af9e..cba70f2f8 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift @@ -77,6 +77,11 @@ extension URLRequest { filename: String, mimeType: String ) throws { + // Validate MIME type matches actual data + guard validateMIMEType(mimeType, matchesData: data) else { + throw EncodingError("MIME type '\(mimeType)' does not match file data") + } + guard var existingBody = httpBody else { throw EncodingError("httpBody not set; call setMultipartFormData first") } @@ -127,4 +132,32 @@ extension URLRequest { self.what = what } } + + private func validateMIMEType(_ mimeType: String, matchesData data: Data) -> Bool { + // Check file signature (magic bytes) matches claimed MIME type + guard data.count >= 12 else { return false } + + let bytes = [UInt8](data.prefix(12)) + + switch mimeType.lowercased() { + case "image/jpeg", "image/jpg": + // JPEG: FF D8 FF + return bytes[0] == 0xFF && bytes[1] == 0xD8 && bytes[2] == 0xFF + + case "image/png": + // PNG: 89 50 4E 47 0D 0A 1A 0A + return bytes[0] == 0x89 && bytes[1] == 0x50 && bytes[2] == 0x4E && + bytes[3] == 0x47 && bytes[4] == 0x0D && bytes[5] == 0x0A && + bytes[6] == 0x1A && bytes[7] == 0x0A + + case "image/gif": + // GIF: 47 49 46 38 (GIF8) + return bytes[0] == 0x47 && bytes[1] == 0x49 && bytes[2] == 0x46 && + bytes[3] == 0x38 && (bytes[4] == 0x37 || bytes[4] == 0x39) // GIF87a or GIF89a + + default: + // For unknown MIME types, allow but log warning + return true + } + } } From 1da1008c114713498b73ff29191a033bd224e759 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 16:16:16 +1100 Subject: [PATCH 11/13] Final Final Refactor (2) v1 --- App/Composition/CompositionMenuTree.swift | 186 +++++++++--------- .../AwfulCore/Model/ForumAttachment.swift | 29 ++- .../URLRequest+MultipartFormData.swift | 8 +- 3 files changed, 113 insertions(+), 110 deletions(-) diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 30c4aaf73..0ee85374e 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -16,8 +16,6 @@ private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: /// Can take over UIMenuController to show a tree of composition-related items on behalf of a text view. final class CompositionMenuTree: NSObject { - // Manages composition menu items and acts as image picker delegate for attachment handling. - @FoilDefaultStorage(Settings.imgurUploadMode) private var imgurUploadMode fileprivate var imgurUploadsEnabled: Bool { @@ -357,50 +355,66 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont isProcessingImage = true let attachment = ForumAttachment(image: image, photoAssetIdentifier: pendingImageAssetIdentifier) - if let error = attachment.validationError { - // Check if the error can be fixed by resizing - let canResize = switch error { - case .fileTooLarge, .dimensionsTooLarge: true - default: false - } - if canResize { - guard let viewController = textView.nearestViewController else { return } - presentAlert( - in: viewController, - title: "Attachment Too Large", - message: "\(error.localizedDescription)\n\nWould you like to automatically resize the image to fit?", - actions: [ - ("Cancel", .cancel, { [weak self] in - self?.clearPendingImage() - }), - ("Resize & Continue", .default, { [weak self] in - self?.resizeAndAttachPendingImage() - }) - ] - ) - } else { - guard let viewController = textView.nearestViewController else { return } - presentAlert( - in: viewController, - title: "Invalid Attachment", - message: error.localizedDescription, - actions: [("OK", .default, nil)] - ) - clearPendingImage() - } + if let error = attachment.validationError { + handleAttachmentValidationError(error) return } - draft?.forumAttachment = attachment + applyAttachmentAndClear(attachment) + } + + private func handleAttachmentValidationError(_ error: ForumAttachment.ValidationError) { + if canResizeToFix(error) { + presentResizePrompt(for: error) + } else { + presentValidationErrorAlert(error) + } + } + + private func canResizeToFix(_ error: ForumAttachment.ValidationError) -> Bool { + switch error { + case .fileTooLarge, .dimensionsTooLarge: return true + default: return false + } + } + + private func presentResizePrompt(for error: ForumAttachment.ValidationError) { + guard let viewController = textView.nearestViewController else { return } + presentAlert( + in: viewController, + title: "Attachment Too Large", + message: "\(error.localizedDescription)\n\nWould you like to automatically resize the image to fit?", + actions: [ + ("Cancel", .cancel, { [weak self] in + self?.clearPendingImage() + }), + ("Resize & Continue", .default, { [weak self] in + self?.resizeAndAttachPendingImage() + }) + ] + ) + } + + private func presentValidationErrorAlert(_ error: ForumAttachment.ValidationError) { + guard let viewController = textView.nearestViewController else { return } + presentAlert( + in: viewController, + title: "Invalid Attachment", + message: error.localizedDescription, + actions: [("OK", .default, nil)] + ) clearPendingImage() + } + private func applyAttachmentAndClear(_ attachment: ForumAttachment) { + draft?.forumAttachment = attachment + clearPendingImage() onAttachmentChanged?() } private func resizeAndAttachPendingImage() { guard let image = pendingImage, let assetID = pendingImageAssetIdentifier else { - // Use weak capture to prevent retaining the full image if not needed guard let image = pendingImage else { return } resizeAndAttach(image: image, assetIdentifier: nil) return @@ -409,76 +423,60 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } private func resizeAndAttach(image: UIImage, assetIdentifier: String?) { - // Show resizing placeholder immediately onResizingStarted?() - // Perform resize in background with weak image reference to prevent memory leak imageProcessingQueue.async { [weak self] in guard let self = self else { return } - let resizedAttachment = autoreleasepool { () -> ForumAttachment? in - let originalAttachment = ForumAttachment(image: image, photoAssetIdentifier: assetIdentifier) - return originalAttachment.resized() + let resizedAttachment = autoreleasepool { + ForumAttachment(image: image, photoAssetIdentifier: assetIdentifier).resized() } DispatchQueue.main.async { [weak self] in - guard let self = self else { return } - - guard let resizedAttachment = resizedAttachment else { - // Don't clear pendingImage - allow retry - self.isProcessingImage = false - // Clear placeholder on failure - self.onAttachmentChanged?() - - guard let viewController = self.textView.nearestViewController else { return } - self.presentAlert( - in: viewController, - title: "Resize Failed", - message: "Unable to resize image to meet requirements.", - actions: [ - ("Try Again", .default, { [weak self] in - self?.resizeAndAttachPendingImage() - }), - ("Cancel", .cancel, { [weak self] in - self?.clearPendingImage() - self?.onAttachmentChanged?() - }) - ] - ) - return - } + self?.handleResizeResult(resizedAttachment) + } + } + } - if let error = resizedAttachment.validationError { - // Don't clear pendingImage - allow retry - self.isProcessingImage = false - // Clear placeholder on validation failure - self.onAttachmentChanged?() - - guard let viewController = self.textView.nearestViewController else { return } - self.presentAlert( - in: viewController, - title: "Resize Failed", - message: "\(error.localizedDescription)", - actions: [ - ("Try Again", .default, { [weak self] in - self?.resizeAndAttachPendingImage() - }), - ("Cancel", .cancel, { [weak self] in - self?.clearPendingImage() - self?.onAttachmentChanged?() - }) - ] - ) - return - } + private func handleResizeResult(_ resizedAttachment: ForumAttachment?) { + guard let attachment = resizedAttachment else { + handleResizeFailure(message: "Unable to resize image to meet requirements.") + return + } - // Success - now we can clear the pending image - self.clearPendingImage() - // Update with resized attachment - self.draft?.forumAttachment = resizedAttachment - self.onAttachmentChanged?() - } + if let error = attachment.validationError { + handleResizeFailure(message: error.localizedDescription) + return } + + handleResizeSuccess(attachment) + } + + private func handleResizeFailure(message: String) { + isProcessingImage = false + onAttachmentChanged?() + + guard let viewController = textView.nearestViewController else { return } + presentAlert( + in: viewController, + title: "Resize Failed", + message: message, + actions: [ + ("Try Again", .default, { [weak self] in + self?.resizeAndAttachPendingImage() + }), + ("Cancel", .cancel, { [weak self] in + self?.clearPendingImage() + self?.onAttachmentChanged?() + }) + ] + ) + } + + private func handleResizeSuccess(_ attachment: ForumAttachment) { + clearPendingImage() + draft?.forumAttachment = attachment + onAttachmentChanged?() } } diff --git a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift index 92c820538..916876db0 100644 --- a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift +++ b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift @@ -41,15 +41,10 @@ public final class ForumAttachment: NSObject, NSCoding { public static let supportedExtensions = ["gif", "jpg", "jpeg", "png"] private struct CompressionSettings { - /// Initial JPEG compression quality (0.9 provides good balance between file size and visual quality) static let defaultQuality: CGFloat = 0.9 - /// Amount to reduce quality on each compression iteration (0.1 provides smooth quality degradation) static let qualityDecrement: CGFloat = 0.1 - /// Minimum acceptable JPEG quality before switching to dimension reduction (0.1 prevents over-compression artifacts) static let minQuality: CGFloat = 0.1 - /// Factor to scale down dimensions for PNG images with alpha (0.9 provides gradual size reduction) static let dimensionScaleFactor: CGFloat = 0.9 - /// Minimum image dimension to maintain readability (100px ensures text/details remain visible) static let minimumDimension = 100 } @@ -79,18 +74,30 @@ public final class ForumAttachment: NSObject, NSCoding { let options = PHImageRequestOptions() options.isSynchronous = true options.deliveryMode = .highQualityFormat + options.isNetworkAccessAllowed = true + var resultImage: UIImage? var requestError: Error? + var timedOut = false + + let semaphore = DispatchSemaphore(value: 0) + PHImageManager.default().requestImage(for: asset, targetSize: PHImageManagerMaximumSize, contentMode: .default, options: options) { image, info in resultImage = image if let error = info?[PHImageErrorKey] as? Error { requestError = error } + semaphore.signal() + } + + if semaphore.wait(timeout: .now() + 30) == .timedOut { + timedOut = true + logger.error("Photo library request timed out for asset: \(photoAssetIdentifier as String)") } if let error = requestError { logger.error("Failed to load image from photo library asset: \(error.localizedDescription)") - } else if resultImage == nil { + } else if resultImage == nil && !timedOut { logger.error("Photo library request returned nil image for asset: \(photoAssetIdentifier as String)") } @@ -247,12 +254,16 @@ public final class ForumAttachment: NSObject, NSCoding { } extension ForumAttachment.ValidationError { + private static let byteFormatter: ByteCountFormatter = { + let formatter = ByteCountFormatter() + formatter.countStyle = .file + return formatter + }() + public var localizedDescription: String { switch self { case .fileTooLarge(let actualSize, let maxSize): - let formatter = ByteCountFormatter() - formatter.countStyle = .file - return "File size (\(formatter.string(fromByteCount: Int64(actualSize)))) exceeds maximum (\(formatter.string(fromByteCount: Int64(maxSize))))" + return "File size (\(Self.byteFormatter.string(fromByteCount: Int64(actualSize)))) exceeds maximum (\(Self.byteFormatter.string(fromByteCount: Int64(maxSize))))" case .dimensionsTooLarge(let width, let height, let maxDimension): return "Image dimensions (\(width)×\(height)) exceed maximum (\(maxDimension)×\(maxDimension))" case .unsupportedFormat: diff --git a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift index cba70f2f8..88ecae98e 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift @@ -77,7 +77,6 @@ extension URLRequest { filename: String, mimeType: String ) throws { - // Validate MIME type matches actual data guard validateMIMEType(mimeType, matchesData: data) else { throw EncodingError("MIME type '\(mimeType)' does not match file data") } @@ -134,29 +133,24 @@ extension URLRequest { } private func validateMIMEType(_ mimeType: String, matchesData data: Data) -> Bool { - // Check file signature (magic bytes) matches claimed MIME type guard data.count >= 12 else { return false } let bytes = [UInt8](data.prefix(12)) switch mimeType.lowercased() { case "image/jpeg", "image/jpg": - // JPEG: FF D8 FF return bytes[0] == 0xFF && bytes[1] == 0xD8 && bytes[2] == 0xFF case "image/png": - // PNG: 89 50 4E 47 0D 0A 1A 0A return bytes[0] == 0x89 && bytes[1] == 0x50 && bytes[2] == 0x4E && bytes[3] == 0x47 && bytes[4] == 0x0D && bytes[5] == 0x0A && bytes[6] == 0x1A && bytes[7] == 0x0A case "image/gif": - // GIF: 47 49 46 38 (GIF8) return bytes[0] == 0x47 && bytes[1] == 0x49 && bytes[2] == 0x46 && - bytes[3] == 0x38 && (bytes[4] == 0x37 || bytes[4] == 0x39) // GIF87a or GIF89a + bytes[3] == 0x38 && (bytes[4] == 0x37 || bytes[4] == 0x39) default: - // For unknown MIME types, allow but log warning return true } } From 4dc4c19f7a5d349da18edaed150e7917e0d06891 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 16:29:24 +1100 Subject: [PATCH 12/13] Refactoring --- App/Composition/CompositionMenuTree.swift | 36 ++++++++++--------- App/Posts/ReplyWorkspace.swift | 5 ++- .../AwfulCore/Model/ForumAttachment.swift | 27 ++++++-------- .../AwfulCore/Networking/ForumsClient.swift | 11 +++--- 4 files changed, 39 insertions(+), 40 deletions(-) diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index 0ee85374e..bc11c4ac6 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -30,21 +30,28 @@ final class CompositionMenuTree: NSObject { private let imageProcessingQueue = DispatchQueue(label: "com.awful.attachment.processing", qos: .userInitiated) private var pendingImage: UIImage? private var pendingImageAssetIdentifier: String? + private let imageProcessingLock = NSLock() private var _isProcessingImage = false - private var isProcessingImage: Bool { - get { - imageProcessingQueue.sync { _isProcessingImage } - } - set { - imageProcessingQueue.sync { _isProcessingImage = newValue } - } + private func tryStartProcessing() -> Bool { + imageProcessingLock.lock() + defer { imageProcessingLock.unlock() } + + guard !_isProcessingImage else { return false } + _isProcessingImage = true + return true + } + + private func finishProcessing() { + imageProcessingLock.lock() + defer { imageProcessingLock.unlock() } + _isProcessingImage = false } private func clearPendingImage() { pendingImage = nil pendingImageAssetIdentifier = nil - isProcessingImage = false + finishProcessing() } /// The textView's class will have some responder chain methods swizzled. @@ -348,12 +355,11 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont fileprivate func useForumAttachmentForPendingImage() { guard let image = pendingImage else { return } - guard !isProcessingImage else { + guard tryStartProcessing() else { logger.warning("Image already being processed, ignoring concurrent request") return } - isProcessingImage = true let attachment = ForumAttachment(image: image, photoAssetIdentifier: pendingImageAssetIdentifier) if let error = attachment.validationError { @@ -414,12 +420,8 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } private func resizeAndAttachPendingImage() { - guard let image = pendingImage, let assetID = pendingImageAssetIdentifier else { - guard let image = pendingImage else { return } - resizeAndAttach(image: image, assetIdentifier: nil) - return - } - resizeAndAttach(image: image, assetIdentifier: assetID) + guard let image = pendingImage else { return } + resizeAndAttach(image: image, assetIdentifier: pendingImageAssetIdentifier) } private func resizeAndAttach(image: UIImage, assetIdentifier: String?) { @@ -453,7 +455,7 @@ extension CompositionMenuTree: UIImagePickerControllerDelegate, UINavigationCont } private func handleResizeFailure(message: String) { - isProcessingImage = false + finishProcessing() onAttachmentChanged?() guard let viewController = textView.nearestViewController else { return } diff --git a/App/Posts/ReplyWorkspace.swift b/App/Posts/ReplyWorkspace.swift index 2a65c8548..59806300b 100644 --- a/App/Posts/ReplyWorkspace.swift +++ b/App/Posts/ReplyWorkspace.swift @@ -137,7 +137,10 @@ final class ReplyWorkspace: NSObject { textView.spellCheckingType = tweaks.spellCheckingType } - compositionViewController.setDraft(draft) + DispatchQueue.main.async { [weak self] in + guard let self = self else { return } + self.compositionViewController.setDraft(self.draft) + } } } diff --git a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift index 916876db0..50ea2d532 100644 --- a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift +++ b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift @@ -77,27 +77,15 @@ public final class ForumAttachment: NSObject, NSCoding { options.isNetworkAccessAllowed = true var resultImage: UIImage? - var requestError: Error? - var timedOut = false - - let semaphore = DispatchSemaphore(value: 0) PHImageManager.default().requestImage(for: asset, targetSize: PHImageManagerMaximumSize, contentMode: .default, options: options) { image, info in resultImage = image if let error = info?[PHImageErrorKey] as? Error { - requestError = error + logger.error("Failed to load image from photo library asset: \(error.localizedDescription)") } - semaphore.signal() - } - - if semaphore.wait(timeout: .now() + 30) == .timedOut { - timedOut = true - logger.error("Photo library request timed out for asset: \(photoAssetIdentifier as String)") } - if let error = requestError { - logger.error("Failed to load image from photo library asset: \(error.localizedDescription)") - } else if resultImage == nil && !timedOut { + if resultImage == nil { logger.error("Photo library request returned nil image for asset: \(photoAssetIdentifier as String)") } @@ -222,6 +210,7 @@ public final class ForumAttachment: NSObject, NSCoding { var currentWidth = targetWidth var currentHeight = targetHeight var resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) + var lastValidImage: UIImage? while compressionQuality > CompressionSettings.minQuality { let hasAlpha = resizedImage.hasAlpha @@ -240,16 +229,22 @@ public final class ForumAttachment: NSObject, NSCoding { if hasAlpha { let newWidth = Int(CGFloat(currentWidth) * CompressionSettings.dimensionScaleFactor) let newHeight = Int(CGFloat(currentHeight) * CompressionSettings.dimensionScaleFactor) - if newWidth < CompressionSettings.minimumDimension || newHeight < CompressionSettings.minimumDimension { break } + if newWidth < CompressionSettings.minimumDimension || newHeight < CompressionSettings.minimumDimension { + break + } currentWidth = newWidth currentHeight = newHeight resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) + + if let data = resizedImage.pngData(), data.count <= maxFileSize { + lastValidImage = resizedImage + } } else { compressionQuality -= CompressionSettings.qualityDecrement } } - return nil + return lastValidImage } } diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index 80867faa6..e57f64344 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -1150,6 +1150,10 @@ public final class ForumsClient { ]) let (document, _) = try parseHTML(data: data, response: response) + return parseAttachmentInfo(from: document) + } + + private func parseAttachmentInfo(from document: HTMLDocument) -> (id: String, filename: String)? { guard let attachmentLink = document.firstNode(matchingSelector: "a[href*='attachment.php']"), let href = attachmentLink["href"] else { return nil @@ -1175,7 +1179,6 @@ public final class ForumsClient { - Throws: An error if the request fails or no attachment is found. */ public func fetchAttachmentImage(postID: String) async throws -> Data { - // First, get the attachment info from the edit page guard let urlSession else { throw Error.missingURLSession } let (data, response) = try await fetch(method: .get, urlString: "editpost.php", parameters: [ @@ -1184,11 +1187,7 @@ public final class ForumsClient { ]) let (document, _) = try parseHTML(data: data, response: response) - guard let attachmentLink = document.firstNode(matchingSelector: "a[href*='attachment.php']"), - let href = attachmentLink["href"], - let components = URLComponents(string: href), - let queryItems = components.queryItems, - let attachmentID = queryItems.first(where: { $0.name == "attachmentid" })?.value else { + guard let (attachmentID, _) = parseAttachmentInfo(from: document) else { throw NSError(domain: "Awful", code: 0, userInfo: [ NSLocalizedDescriptionKey: "No attachment found for this post" ]) From 0e2c7b68e76f7b77e5227b1f149eb8d97c8b345d Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 15 Nov 2025 16:43:22 +1100 Subject: [PATCH 13/13] The final refactor for real --- App/Composition/AttachmentCardView.swift | 8 ++++ App/Composition/AttachmentEditView.swift | 2 +- App/Composition/AttachmentPreviewView.swift | 2 +- App/Composition/CompositionMenuTree.swift | 5 +++ App/Extensions/HTMLReader.swift | 6 +-- .../AwfulCore/Model/ForumAttachment.swift | 39 ++++++++++++++++++- .../AwfulCore/Networking/ForumsClient.swift | 14 +------ .../URLRequest+MultipartFormData.swift | 7 +++- .../Sources/AwfulCore/Scraping/Helpers.swift | 18 +++++++++ 9 files changed, 81 insertions(+), 20 deletions(-) diff --git a/App/Composition/AttachmentCardView.swift b/App/Composition/AttachmentCardView.swift index 7a72c1bb9..a30e42e7d 100644 --- a/App/Composition/AttachmentCardView.swift +++ b/App/Composition/AttachmentCardView.swift @@ -6,13 +6,21 @@ import UIKit /// Shared layout constants for attachment card views enum AttachmentCardLayout { + /// Size of the thumbnail image (width and height) static let imageSize: CGFloat = 60 + /// Corner radius for the thumbnail image static let imageCornerRadius: CGFloat = 4 + /// Corner radius for the card container static let cardCornerRadius: CGFloat = 8 + /// Padding around the card edges static let cardPadding: CGFloat = 12 + /// Spacing between image and text labels static let imageSpacing: CGFloat = 12 + /// Top padding for labels (larger to optically center with image) static let labelTopPadding: CGFloat = 16 + /// Spacing between title and detail labels static let titleDetailSpacing: CGFloat = 4 + /// Size of action buttons (remove, etc.) static let actionButtonSize: CGFloat = 30 } diff --git a/App/Composition/AttachmentEditView.swift b/App/Composition/AttachmentEditView.swift index 31353d5d5..2e366d937 100644 --- a/App/Composition/AttachmentEditView.swift +++ b/App/Composition/AttachmentEditView.swift @@ -72,7 +72,7 @@ final class AttachmentEditView: AttachmentCardView { actionSegmentedControl.leadingAnchor.constraint(equalTo: leadingAnchor, constant: AttachmentCardLayout.cardPadding), actionSegmentedControl.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -AttachmentCardLayout.cardPadding), actionSegmentedControl.topAnchor.constraint(equalTo: imageView.bottomAnchor, constant: AttachmentCardLayout.imageSpacing), - actionSegmentedControl.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -AttachmentCardLayout.cardPadding) + actionSegmentedControl.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -AttachmentCardLayout.cardPadding), ]) } diff --git a/App/Composition/AttachmentPreviewView.swift b/App/Composition/AttachmentPreviewView.swift index ff7904b7d..61e75b881 100644 --- a/App/Composition/AttachmentPreviewView.swift +++ b/App/Composition/AttachmentPreviewView.swift @@ -68,7 +68,7 @@ final class AttachmentPreviewView: AttachmentCardView { removeButton.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -AttachmentCardLayout.cardPadding), removeButton.centerYAnchor.constraint(equalTo: centerYAnchor), removeButton.widthAnchor.constraint(equalToConstant: AttachmentCardLayout.actionButtonSize), - removeButton.heightAnchor.constraint(equalToConstant: AttachmentCardLayout.actionButtonSize) + removeButton.heightAnchor.constraint(equalToConstant: AttachmentCardLayout.actionButtonSize), ]) } diff --git a/App/Composition/CompositionMenuTree.swift b/App/Composition/CompositionMenuTree.swift index bc11c4ac6..c9dd3e41e 100644 --- a/App/Composition/CompositionMenuTree.swift +++ b/App/Composition/CompositionMenuTree.swift @@ -28,8 +28,13 @@ final class CompositionMenuTree: NSObject { var onResizingStarted: (() -> Void)? private let imageProcessingQueue = DispatchQueue(label: "com.awful.attachment.processing", qos: .userInitiated) + + // Thread safety: pendingImage properties are only accessed from the main thread during image picker flow. + // They are set on the main thread before dispatching to imageProcessingQueue, and cleared on the main + // thread after processing completes. No lock is needed as there's no concurrent access. private var pendingImage: UIImage? private var pendingImageAssetIdentifier: String? + private let imageProcessingLock = NSLock() private var _isProcessingImage = false diff --git a/App/Extensions/HTMLReader.swift b/App/Extensions/HTMLReader.swift index f170be5ec..4c05cf353 100644 --- a/App/Extensions/HTMLReader.swift +++ b/App/Extensions/HTMLReader.swift @@ -36,11 +36,11 @@ extension HTMLTextNode { /** Splits the text node in two or three, isolating the text at `range` in its own node, and replaces the text node with the newly-created text nodes. - + If the range covers the entire text node, it is returned unchanged. - + If the text node has no parent, returned text nodes will also have no parent. - + It is a programmer error if `range` exceeds the bounds of the text node's data. */ func split(_ range: Range) -> SplitRangeResult { diff --git a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift index 50ea2d532..95834f50d 100644 --- a/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift +++ b/AwfulCore/Sources/AwfulCore/Model/ForumAttachment.swift @@ -66,15 +66,27 @@ public final class ForumAttachment: NSObject, NSCoding { self.validationError = validate() } + /** + Restores a ForumAttachment from encoded state. + + - Important: This initializer performs synchronous photo library requests when restoring from a photo asset. + During state restoration, this blocking behavior is acceptable as it ensures the attachment is fully + loaded before the UI is presented. The synchronous request is configured to allow network access, + so images stored in iCloud Photos may incur network delays. + + - Parameter coder: The decoder to read data from + + - Returns: A restored ForumAttachment, or nil if decoding fails + */ public required init?(coder: NSCoder) { if let photoAssetIdentifier = coder.decodeObject(of: NSString.self, forKey: CodingKeys.assetIdentifier.rawValue) { self.photoAssetIdentifier = photoAssetIdentifier as String if let asset = PHAsset.fetchAssets(withLocalIdentifiers: [photoAssetIdentifier as String], options: nil).firstObject { let options = PHImageRequestOptions() - options.isSynchronous = true + options.isSynchronous = true // Blocking call - acceptable during state restoration options.deliveryMode = .highQualityFormat - options.isNetworkAccessAllowed = true + options.isNetworkAccessAllowed = true // May fetch from iCloud if needed var resultImage: UIImage? @@ -200,6 +212,23 @@ public final class ForumAttachment: NSObject, NSCoding { return nil } + /** + Compresses an image to fit within the specified dimensions and file size limits. + + The compression algorithm uses a two-pronged approach: + - For images without alpha channel: Uses JPEG compression with progressively decreasing quality + - For images with alpha channel: Uses PNG compression with progressively smaller dimensions + + This strategy optimizes for file size while preserving visual quality and transparency where needed. + + - Parameters: + - originalImage: The source image to compress + - targetWidth: Maximum width in pixels + - targetHeight: Maximum height in pixels + - maxFileSize: Maximum file size in bytes + + - Returns: A compressed image that fits within the constraints, or `nil` if compression failed + */ private func compressImage( _ originalImage: UIImage, targetWidth: Int, @@ -212,20 +241,24 @@ public final class ForumAttachment: NSObject, NSCoding { var resizedImage = originalImage.resized(to: CGSize(width: currentWidth, height: currentHeight)) var lastValidImage: UIImage? + // Iteratively compress until we meet the file size constraint while compressionQuality > CompressionSettings.minQuality { let hasAlpha = resizedImage.hasAlpha let data: Data? + // Choose encoding based on alpha channel presence if hasAlpha { data = resizedImage.pngData() } else { data = resizedImage.jpegData(compressionQuality: compressionQuality) } + // Success: image fits within file size limit if let imageData = data, imageData.count <= maxFileSize { return resizedImage } + // For images with alpha: reduce dimensions (PNG doesn't support quality adjustment) if hasAlpha { let newWidth = Int(CGFloat(currentWidth) * CompressionSettings.dimensionScaleFactor) let newHeight = Int(CGFloat(currentHeight) * CompressionSettings.dimensionScaleFactor) @@ -240,10 +273,12 @@ public final class ForumAttachment: NSObject, NSCoding { lastValidImage = resizedImage } } else { + // For images without alpha: reduce JPEG quality compressionQuality -= CompressionSettings.qualityDecrement } } + // Return the last image that fit within constraints, or nil if none did return lastValidImage } } diff --git a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift index e57f64344..475986d18 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/ForumsClient.swift @@ -978,7 +978,7 @@ public final class ForumsClient { private func parseAttachmentLimits(from document: HTMLDocument) -> (maxFileSize: Int, maxDimension: Int)? { guard let maxFileSizeString = document.firstNode(matchingSelector: "input[name='MAX_FILE_SIZE']")?["value"], let maxFileSize = Int(maxFileSizeString) else { - logger.debug("Could not parse MAX_FILE_SIZE from HTML, falling back to defaults") + logger.warning("Could not parse MAX_FILE_SIZE from HTML, falling back to defaults") return nil } @@ -987,17 +987,7 @@ public final class ForumsClient { for td in document.nodes(matchingSelector: "form[name='vbform'] td") { let text = td.textContent if text.contains("Attach file:") { - var nextSibling: HTMLElement? - if let sibling = td.nextSibling as? HTMLElement { - nextSibling = sibling - } else if let parent = td.parent { - let children = parent.children.compactMap { $0 as? HTMLElement } - if let index = children.firstIndex(where: { $0 === td }), index + 1 < children.count { - nextSibling = children[index + 1] - } - } - - if let nextSibling = nextSibling { + if let nextSibling = td.nextSiblingElement { let limitsText = nextSibling.textContent let pattern = #"dimensions:\s*(\d+)x(\d+)"# do { diff --git a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift index 88ecae98e..2cf1b23ae 100644 --- a/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift +++ b/AwfulCore/Sources/AwfulCore/Networking/URLRequest+MultipartFormData.swift @@ -3,6 +3,9 @@ // Copyright 2023 Awful Contributors. CC BY-NC-SA 3.0 US https://github.com/Awful/Awful.app import Foundation +import os.log + +private let logger = Logger(subsystem: "com.awful.Awful", category: "MultipartFormData") extension URLRequest { /** @@ -151,7 +154,9 @@ extension URLRequest { bytes[3] == 0x38 && (bytes[4] == 0x37 || bytes[4] == 0x39) default: - return true + // Unknown MIME type - log warning and reject for security + logger.warning("Attempted to upload file with unknown MIME type: \(mimeType)") + return false } } } diff --git a/AwfulCore/Sources/AwfulCore/Scraping/Helpers.swift b/AwfulCore/Sources/AwfulCore/Scraping/Helpers.swift index 7ceb3e5a5..fef505d40 100644 --- a/AwfulCore/Sources/AwfulCore/Scraping/Helpers.swift +++ b/AwfulCore/Sources/AwfulCore/Scraping/Helpers.swift @@ -35,6 +35,24 @@ extension HTMLElement { .components(separatedBy: .asciiWhitespace) .filter { !$0.isEmpty } } + + /** + Returns the next sibling element in the DOM tree. + + This helper method handles cases where the next sibling might be a text node or other non-element node, + returning the first element sibling found. + */ + var nextSiblingElement: HTMLElement? { + if let sibling = nextSibling as? HTMLElement { + return sibling + } else if let parent = parent { + let children = parent.children.compactMap { $0 as? HTMLElement } + if let index = children.firstIndex(where: { $0 === self }), index + 1 < children.count { + return children[index + 1] + } + } + return nil + } }