-
Notifications
You must be signed in to change notification settings - Fork 195
feat(pdf): add highlight support with anchor extraction and resolution #705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
e881e40
70f9621
9dfa6e6
d9e5770
11ed9c5
61bbf79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -135,6 +135,18 @@ final class EditingActionsController { | |
| return delegate?.editingActions(self, canPerformAction: action, for: selection) ?? true | ||
| } | ||
|
|
||
| /// Checks whether a given selector is handled by any of the configured editing actions. | ||
| /// | ||
| /// This is useful for custom responder chain handling, particularly in PDF views | ||
| /// where you need to distinguish between actions managed by this controller | ||
| /// and system actions. | ||
| /// | ||
| /// - Parameter selector: The selector to check. | ||
| /// - Returns: `true` if any editing action handles this selector, `false` otherwise. | ||
| func handlesAction(_ selector: Selector) -> Bool { | ||
| actions.contains { $0.actions.contains(selector) } | ||
| } | ||
|
|
||
| /// Verifies that the user has the rights to use the given `action`. | ||
| private func isActionAllowed(_ action: EditingAction) -> Bool { | ||
| switch action { | ||
|
|
@@ -158,9 +170,76 @@ final class EditingActionsController { | |
| // Expansion setting which allows to copy the selection. | ||
| // To reproduce, comment out and select Japanese text on a PDF. | ||
| builder.remove(menu: .learn) | ||
|
|
||
| // iOS 16+ enhancement: Add custom actions as UICommand items to the edit menu. | ||
| // This ensures custom actions (like "Highlight") appear properly in the modern | ||
| // UIEditMenuInteraction on iOS 16+, fixing the issue where custom actions | ||
| // wouldn't show up or wouldn't route correctly to the responder chain. | ||
| if #available(iOS 16.0, *) { | ||
| addCustomActionsToMenu(builder) | ||
| } | ||
| } | ||
|
|
||
| /// Adds custom editing actions to the menu builder for iOS 16+. | ||
| /// | ||
| /// This method converts custom `UIMenuItem` actions into `UICommand` items, | ||
| /// which properly integrate with iOS 16's `UIEditMenuInteraction` system. | ||
| /// It maintains backward compatibility by only affecting iOS 16+ behavior. | ||
| @available(iOS 16.0, *) | ||
| private func addCustomActionsToMenu(_ builder: UIMenuBuilder) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, we will be able to remove the iOS 15 system entirely next year when we bump to iOS 16 👍 |
||
| // Extract custom actions and convert them to UICommand | ||
| let customElements: [UIMenuElement] = actions.compactMap { action in | ||
| switch action.kind { | ||
| case let .custom(menuItem): | ||
| return UICommand( | ||
| title: menuItem.title, | ||
| image: nil, | ||
| action: menuItem.action, | ||
| propertyList: nil | ||
| ) | ||
| case .native: | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| guard !customElements.isEmpty else { | ||
| return | ||
| } | ||
|
|
||
| // Check if we have any native actions (copy, lookup, translate, share) | ||
| let hasNativeActions = actions.contains { | ||
| if case .native = $0.kind { | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // Get existing menu items | ||
| var standardChildren = builder.menu(for: .standardEdit)?.children ?? [] | ||
|
|
||
| if hasNativeActions { | ||
| // If we have native actions, prepend custom actions to preserve | ||
| // standard system actions (copy, lookup, etc.) | ||
| standardChildren.insert(contentsOf: customElements, at: 0) | ||
| } else { | ||
| // If only custom actions, replace the menu to avoid clutter | ||
| // This gives integrators full control when they don't use native actions | ||
| standardChildren = customElements | ||
| } | ||
|
|
||
| builder.replaceChildren(ofMenu: .standardEdit) { _ in standardChildren } | ||
| } | ||
|
|
||
| func updateSharedMenuController() { | ||
| if #available(iOS 16.0, *) { | ||
| // On iOS 16+, UIMenuController is deprecated in favor of UIEditMenuInteraction. | ||
| // Clear the shared menu items to avoid conflicts between the old and new systems. | ||
| // Custom actions are now handled via buildMenu(with:) and UICommand. | ||
| UIMenuController.shared.menuItems = nil | ||
| return | ||
| } | ||
|
|
||
| // iOS 15 and earlier: Use legacy UIMenuController system | ||
| var items: [UIMenuItem] = [] | ||
| if isEnabled, let selection = selection { | ||
| items = actions | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| // | ||
| // Copyright 2026 Readium Foundation. All rights reserved. | ||
| // Use of this source code is governed by the BSD-style license | ||
| // available in the top-level LICENSE file of the project. | ||
| // | ||
|
|
||
| import Foundation | ||
| import PDFKit | ||
| import ReadiumShared | ||
|
|
||
| /// Extracts precise anchor data from a PDF selection for highlight persistence. | ||
| /// | ||
| /// The extractor captures multiple forms of positioning data: | ||
| /// - Coordinate quads for pixel-perfect rendering | ||
| /// - Character ranges for text-based lookup | ||
| /// - Surrounding text context for disambiguation | ||
| public struct PDFAnchorExtractor: Loggable { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't look like this API needs to be public? |
||
|
|
||
| /// Number of characters to capture before/after the selection for context. | ||
| public static let contextCharacterCount = 20 | ||
|
|
||
| /// Extracts anchor data from a PDF selection. | ||
| /// | ||
| /// - Parameters: | ||
| /// - selection: The PDF selection to extract anchor data from. | ||
| /// - page: The page containing the selection. | ||
| /// - Returns: A dictionary suitable for storage in Locator.Locations.otherLocations, | ||
| /// or nil if extraction fails. | ||
| public static func extractAnchor( | ||
| from selection: PDFSelection, | ||
| on page: PDFPage | ||
| ) -> [String: Any]? { | ||
| guard let pageIndex = page.document?.index(for: page), | ||
| let selectedText = selection.string, | ||
| !selectedText.isEmpty | ||
| else { | ||
| log(.debug, "PDF anchor extraction skipped: invalid selection") | ||
| return nil | ||
| } | ||
|
|
||
| var anchor: [String: Any] = [ | ||
| "pageIndex": pageIndex, | ||
| "text": selectedText | ||
| ] | ||
|
|
||
| // Extract coordinate quads | ||
| if let quads = extractQuads(from: selection, on: page) { | ||
| anchor["quads"] = quads | ||
| } | ||
|
|
||
| // Extract character range | ||
| if let pageText = page.string, | ||
| let range = extractCharacterRange(for: selectedText, in: pageText, selection: selection, page: page) { | ||
| anchor["characterStart"] = range.lowerBound | ||
| anchor["characterEnd"] = range.upperBound | ||
|
|
||
| // Extract context | ||
| let (before, after) = extractContext( | ||
| around: range, | ||
| in: pageText, | ||
| contextLength: contextCharacterCount | ||
| ) | ||
| if let before = before { | ||
| anchor["textBefore"] = before | ||
| } | ||
| if let after = after { | ||
| anchor["textAfter"] = after | ||
| } | ||
|
Comment on lines
+63
to
+68
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These data should be part of the |
||
| } | ||
|
|
||
| log(.debug, "PDF anchor extracted: page=\(pageIndex), chars=\(anchor["characterStart"] ?? "nil")-\(anchor["characterEnd"] ?? "nil"), quads=\(anchor["quads"] != nil)") | ||
|
|
||
| return anchor | ||
| } | ||
|
|
||
| /// Extracts quadrilateral bounds for each line of the selection. | ||
| private static func extractQuads( | ||
| from selection: PDFSelection, | ||
| on page: PDFPage | ||
| ) -> [[[String: Double]]]? { | ||
| let lineSelections = selection.selectionsByLine() | ||
| guard !lineSelections.isEmpty else { return nil } | ||
|
|
||
| var quads: [[[String: Double]]] = [] | ||
|
|
||
| for lineSelection in lineSelections { | ||
| let bounds = lineSelection.bounds(for: page) | ||
| guard !bounds.isNull, !bounds.isEmpty else { continue } | ||
|
|
||
| // Convert CGRect to quad (4 corner points) | ||
| let quad: [[String: Double]] = [ | ||
| ["x": Double(bounds.minX), "y": Double(bounds.minY)], // bottomLeft | ||
| ["x": Double(bounds.maxX), "y": Double(bounds.minY)], // bottomRight | ||
| ["x": Double(bounds.maxX), "y": Double(bounds.maxY)], // topRight | ||
| ["x": Double(bounds.minX), "y": Double(bounds.maxY)] // topLeft | ||
| ] | ||
| quads.append(quad) | ||
| } | ||
|
|
||
| return quads.isEmpty ? nil : quads | ||
| } | ||
|
|
||
| /// Extracts the character range of the selection within the page text. | ||
| private static func extractCharacterRange( | ||
| for selectedText: String, | ||
| in pageText: String, | ||
| selection: PDFSelection, | ||
| page: PDFPage | ||
| ) -> Range<Int>? { | ||
| // Try to get the range from PDFKit's selection | ||
| // PDFSelection doesn't expose character range directly, so we search | ||
|
|
||
| // Find all occurrences of the selected text | ||
| var ranges: [Range<String.Index>] = [] | ||
| var searchStart = pageText.startIndex | ||
|
|
||
| while let range = pageText.range(of: selectedText, range: searchStart..<pageText.endIndex) { | ||
| ranges.append(range) | ||
| searchStart = range.upperBound | ||
| } | ||
|
|
||
| guard !ranges.isEmpty else { return nil } | ||
|
|
||
| // If only one occurrence, use it | ||
| if ranges.count == 1 { | ||
| let range = ranges[0] | ||
| let start = pageText.distance(from: pageText.startIndex, to: range.lowerBound) | ||
| let end = pageText.distance(from: pageText.startIndex, to: range.upperBound) | ||
| return start..<end | ||
| } | ||
|
|
||
| // Multiple occurrences: try to disambiguate using selection bounds | ||
| let selectionBounds = selection.bounds(for: page) | ||
|
|
||
| for range in ranges { | ||
| let nsRange = NSRange(range, in: pageText) | ||
| if let testSelection = page.selection(for: nsRange) { | ||
| let testBounds = testSelection.bounds(for: page) | ||
| // Check if bounds are approximately equal (within tolerance) | ||
| if boundsApproximatelyEqual(selectionBounds, testBounds, tolerance: 5.0) { | ||
| let start = pageText.distance(from: pageText.startIndex, to: range.lowerBound) | ||
| let end = pageText.distance(from: pageText.startIndex, to: range.upperBound) | ||
| return start..<end | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Fallback: use first occurrence | ||
| let range = ranges[0] | ||
| let start = pageText.distance(from: pageText.startIndex, to: range.lowerBound) | ||
| let end = pageText.distance(from: pageText.startIndex, to: range.upperBound) | ||
| return start..<end | ||
| } | ||
|
|
||
| /// Extracts text context around the given range. | ||
| /// - Note: Internal for testing. | ||
| static func extractContext( | ||
| around range: Range<Int>, | ||
| in text: String, | ||
| contextLength: Int | ||
| ) -> (before: String?, after: String?) { | ||
| let startIndex = text.index(text.startIndex, offsetBy: range.lowerBound) | ||
| let endIndex = text.index(text.startIndex, offsetBy: range.upperBound) | ||
|
|
||
| // Extract before context | ||
| let beforeStart = text.index( | ||
| startIndex, | ||
| offsetBy: -contextLength, | ||
| limitedBy: text.startIndex | ||
| ) ?? text.startIndex | ||
| let before = String(text[beforeStart..<startIndex]) | ||
|
|
||
| // Extract after context | ||
| let afterEnd = text.index( | ||
| endIndex, | ||
| offsetBy: contextLength, | ||
| limitedBy: text.endIndex | ||
| ) ?? text.endIndex | ||
| let after = String(text[endIndex..<afterEnd]) | ||
|
|
||
| return ( | ||
| before: before.isEmpty ? nil : before, | ||
| after: after.isEmpty ? nil : after | ||
| ) | ||
| } | ||
|
|
||
| /// Checks if two bounds are approximately equal within a tolerance. | ||
| /// - Note: Internal for testing. | ||
| static func boundsApproximatelyEqual( | ||
| _ a: CGRect, | ||
| _ b: CGRect, | ||
| tolerance: CGFloat | ||
| ) -> Bool { | ||
| abs(a.minX - b.minX) <= tolerance && | ||
| abs(a.minY - b.minY) <= tolerance && | ||
| abs(a.maxX - b.maxX) <= tolerance && | ||
| abs(a.maxY - b.maxY) <= tolerance | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't you able to use
canPerformAction()for this?