AttributedLabel Accessibility Links (#537)

This PR migrates `AttributedLabel`'s link rotor from a stateful to
functional pattern, eliminating the need for two variables and
preventing voiceover from getting out of sync.

I also fixed a bug in `links(at location: CGPoint)` that could cause a
crash if the `AttributedString` lacked a specified `NSTextAlignment`.

BlueprintUI/Sources/Extensions/BidirectionalCollection+Blueprint.swift

@@ -0,0 +1,22 @@
+import UIKit
+
+extension BidirectionalCollection where Element: Equatable & NSObjectProtocol {
+
+    /// Returns a UIAccessibilityCustomRotor with the provided system type which cycles through the contained elements.
+    public func accessibilityRotor(systemType type: UIAccessibilityCustomRotor.SystemRotorType) -> UIAccessibilityCustomRotor {
+        UIAccessibilityCustomRotor(systemType: type) { itemSearch($0) }
+    }
+
+    private func itemSearch(_ predicate: UIAccessibilityCustomRotorSearchPredicate) -> UIAccessibilityCustomRotorItemResult? {
+        guard let first else { return nil }
+        guard let currentItem = predicate.currentItem.targetElement as? Element,
+              let currentIndex = firstIndex(of: currentItem),
+              predicate.searchDirection == .previous || predicate.searchDirection == .next
+        else {
+            return UIAccessibilityCustomRotorItemResult(targetElement: first, targetRange: nil)
+        }
+        let newIndex = (predicate.searchDirection == .next ? index(after: currentIndex) : index(before: currentIndex))
+        guard newIndex >= startIndex, newIndex < endIndex else { return nil }
+        return .init(targetElement: self[newIndex], targetRange: nil)
+    }
+}

BlueprintUICommonControls/Sources/AttributedLabel.swift

@@ -210,35 +210,11 @@ extension AttributedLabel {
             }
         }
 
-        /// The view needs to keep track of the current index for the accessibility rotor.
-        private var accessibilityLinkIndex = -1
-
-        /// These elements need to be retained by the view, and cannot be created inside the
-        /// `accessibilityCustomRotors` getter.
-        private var accessibilityLinks: [LinkAccessibilityElement] = []
-
         override var accessibilityCustomRotors: [UIAccessibilityCustomRotor]? {
-            set { fatalError() }
+            set { fatalError("accessibilityCustomRotors is not settable.") }
             get {
-                accessibilityLinks.isEmpty
-                    ? []
-                    : [
-                        UIAccessibilityCustomRotor(systemType: .link) { [weak self] predicate in
-                            guard let self = self, !self.accessibilityLinks.isEmpty else {
-                                return nil
-                            }
-
-                            self.accessibilityLinkIndex += predicate.searchDirection == .next ? 1 : -1
-                            self.accessibilityLinkIndex = min(
-                                self.accessibilityLinks.count - 1,
-                                self.accessibilityLinkIndex
-                            )
-                            self.accessibilityLinkIndex = max(0, self.accessibilityLinkIndex)
-
-                            let link = self.accessibilityLinks[self.accessibilityLinkIndex]
-                            return UIAccessibilityCustomRotorItemResult(targetElement: link, targetRange: nil)
-                        },
-                    ]
+                guard let attributedText, !links.isEmpty else { return [] }
+                return [accessibilityRotor(for: links, in: attributedText)]
             }
         }
 
@@ -275,7 +251,6 @@ extension AttributedLabel {
             if !isMeasuring {
                 if previousAttributedText != attributedText {
                     links = attributedLinks(in: model.attributedText) + detectedDataLinks(in: model.attributedText)
-                    accessibilityLinks = accessibilityLinks(for: links, in: model.attributedText)
                     accessibilityLabel = accessibilityLabel(
                         with: links,
                         in: model.attributedText.string,
@@ -389,7 +364,7 @@ extension AttributedLabel {
                 }
 
                 assert(
-                    alignments.count == 1,
+                    alignments.count <= 1,
                     """
                     AttributedLabel links only support a single NSTextAlignment. \
                     Instead, found: \(alignments).
@@ -539,30 +514,28 @@ extension AttributedLabel {
             return links
         }
 
-        private func accessibilityLinks(for links: [Link], in string: NSAttributedString) -> [LinkAccessibilityElement] {
-            links
+        internal func accessibilityRotor(for links: [Link], in string: NSAttributedString) -> UIAccessibilityCustomRotor {
+            let elements: [LinkAccessibilityElement] = links
                 .sorted(by: { $0.range.location < $1.range.location })
                 .compactMap { link in
                     guard NSIntersectionRange(string.entireRange, link.range).length > 0 else {
                         return nil
                     }
-
                     return LinkAccessibilityElement(
                         container: self,
                         label: string.attributedSubstring(from: link.range).string,
                         link: link
                     )
                 }
-
-
+            return elements.accessibilityRotor(systemType: .link)
         }
 
         internal func accessibilityLabel(with links: [Link], in string: String, linkAccessibilityLabel: String?) -> String {
-            // When reading an attributed string that contains the `.link` attribute VoiceOver will announce "link" when it encounters the applied range. This is important because it informs the user about the context and position of the linked text within the greater string. This can be partocularly important when a string contains multiple links with the same linked text but different link destinations.
+            // When reading an attributed string that contains the `.link` attribute VoiceOver will announce "link" when it encounters the applied range. This is important because it informs the user about the context and position of the linked text within the greater string. This can be particularly important when a string contains multiple links with the same linked text but different link destinations.
 
-            // UILabel is extremely insistant about how the `.link` attribute should be styled going so far as to apply its own preferences above any other provided attributes. In order to allow custom link styling we replace any instances of the `.link` attribute with a `labelLink.` attribute (see `NSAttributedString.normalizingForView(with:)`. This allows us to track the location of links while still providing our own custom styling. Unfortunately this means that voiceover doesnt recognize our links as links and consequently they are not announced to the user.
+            // UILabel is extremely insistent about how the `.link` attribute should be styled going so far as to apply its own preferences above any other provided attributes. In order to allow custom link styling we replace any instances of the `.link` attribute with a `labelLink.` attribute (see `NSAttributedString.normalizingForView(with:)`. This allows us to track the location of links while still providing our own custom styling. Unfortunately this means that voiceover doesn't recognize our links as links and consequently they are not announced to the user.
 
-            // Ideally we'd be able to enumerate our links, insert the `.link` attribute back and then set the resulting string as the `accessibilityAttributedString` but unfortunately that doesnt seem to work. Apple's [docs](https://developer.apple.com/documentation/objectivec/nsobject/2865944-accessibilityattributedlabel) indicate that this property is intended "for the inclusion of language attributes in the string to control pronunciation or accents" and doesnt seem to notice any included `.link` attributes.
+            // Ideally we'd be able to enumerate our links, insert the `.link` attribute back and then set the resulting string as the `accessibilityAttributedString` but unfortunately that doesn't seem to work. Apple's [docs](https://developer.apple.com/documentation/objectivec/nsobject/2865944-accessibilityattributedlabel) indicate that this property is intended "for the inclusion of language attributes in the string to control pronunciation or accents" and doesnt seem to notice any included `.link` attributes.
 
             // Insert the word "link" after each link in the label. This mirrors the VoiceOver behavior when encountering a `.link` attribute.
 
@@ -708,7 +681,7 @@ extension AttributedLabel {
         }
 
         override var accessibilityFrameInContainerSpace: CGRect {
-            set { fatalError() }
+            set { fatalError("accessibilityFrameInContainerSpace") }
             get {
                 guard let container = container,
                       let textStorage = container.makeTextStorage(),

BlueprintUICommonControls/Tests/Sources/AttributedLabelTests.swift

@@ -485,6 +485,66 @@ class AttributedLabelTests: XCTestCase {
     }
 
 
+    func test_linkAccessibility_Rotors() {
+        let labelView = AttributedLabel.LabelView()
+        let text: NSString = "The Fellowship of the ring was established at the Council of Elrond and consisted of Gandalf, Sam, Frodo, Aragorn, Gimli, Pippin, Boromir, Legolas, and Merry."
+
+        let url = URL(string: "https://one.ring")!
+
+        let links = ["Frodo", "Merry", "Sam", "Pippin"].map {
+            AttributedLabel.Link(url: url, range: text.range(of: $0))
+        }
+
+        let rotor = labelView.accessibilityRotor(for: links, in: NSAttributedString(string: text as String))
+        XCTAssertNotNil(rotor)
+
+        // links should be sorted by their position in the main string.
+        let sortedHobbits = rotor.dumpItems().map { $0.accessibilityLabel }
+        XCTAssertEqual(sortedHobbits, ["Sam", "Frodo", "Pippin", "Merry"])
+    }
+
+
+
+    func test_linkAccessibility_Rotors_update() {
+        let string = "The Fellowship of the ring was established at the Council of Elrond and consisted of Gandalf, Sam, Frodo, Aragorn, Gimli, Pippin, Boromir, Legolas, and Merry."
+        var attributedText = AttributedText(string)
+
+        for hobbit in ["Frodo", "Merry", "Sam", "Pippin"] {
+            let range = attributedText.range(of: hobbit)!
+            attributedText[range].link = URL(string: "https://one.ring")!
+        }
+
+        var label = AttributedLabel(attributedText: attributedText.attributedString)
+        let labelView = AttributedLabel.LabelView()
+        labelView.update(model: label, text: label.attributedText, environment: .empty, isMeasuring: false)
+
+        let rotor = labelView.accessibilityCustomRotors!.first!
+        XCTAssertNotNil(rotor)
+
+        // links should be sorted by their position in the main string.
+        let sortedHobbits = rotor.dumpItems().map { $0.accessibilityLabel }
+        XCTAssertEqual(sortedHobbits, ["Sam", "Frodo", "Pippin", "Merry"])
+
+        let removedLinks = AttributedText(string)
+        label.attributedText = removedLinks.attributedString
+        labelView.update(model: label, text: label.attributedText, environment: .empty, isMeasuring: false)
+        XCTAssertTrue(labelView.accessibilityCustomRotors!.isEmpty)
+
+        var updatedText = AttributedText(string)
+        for name in ["Aragorn", "Gandalf", "Gimli", "Legolas", "Boromir"] {
+            let range = updatedText.range(of: name)!
+            updatedText[range].link = URL(string: "https://one.ring")!
+        }
+        label.attributedText = updatedText.attributedString
+        labelView.update(model: label, text: label.attributedText, environment: .empty, isMeasuring: false)
+
+        let updatedRotor = labelView.accessibilityCustomRotors!.first!
+        XCTAssertNotNil(updatedRotor)
+
+        let notHobbits = updatedRotor.dumpItems().map { $0.accessibilityLabel }
+        XCTAssertEqual(notHobbits, ["Gandalf", "Aragorn", "Gimli", "Boromir", "Legolas"])
+    }
+
 
     func test_textContainerRects() {
         let lineBreakModes: [NSLineBreakMode?] = [
@@ -654,6 +714,44 @@ class AttributedLabelTests: XCTestCase {
 
 }
 
+extension UIAccessibilityCustomRotor {
+    fileprivate func dumpItems() -> [NSObject] {
+        var results = [UIAccessibilityCustomRotorItemResult]()
+
+        let predicate = UIAccessibilityCustomRotorSearchPredicate()
+        predicate.searchDirection = .next
+
+        let first = itemSearchBlock(predicate)
+        XCTAssertNotNil(first)
+        results = [first!]
+
+
+        predicate.currentItem = first!
+
+        while let last = results.last,
+              let next = itemSearchBlock(predicate),
+              last.targetElement as? NSObject != next.targetElement as? NSObject
+        {
+            results = results + [next]
+            predicate.currentItem = next
+        }
+
+        predicate.searchDirection = .previous
+        predicate.currentItem = first!
+
+        while let last = results.first,
+              let next = itemSearchBlock(predicate),
+              last.targetElement as? NSObject != next.targetElement as? NSObject
+        {
+            results = [next] + results
+            predicate.currentItem = next
+        }
+
+
+        return results.compactMap { $0.targetElement as? NSObject }
+    }
+}
+
 fileprivate struct LabelTapDetectionGrid: Element {
 
     var wrapping: Element

CHANGELOG.md

@@ -9,7 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Fixed a bug in `AttributedLabel` which could cause a crash if the attributed string lacked a specified `NSTextAlignment`. 
+
 ### Added
+
 - `AccessibilityContainer` now supports configuration of `UIAccessibilityContainerType`, `AccessibilityLabel` and `AccessibilityValue`.
 
 ### Removed
@@ -18,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+`AttributedLabel` accessibility links are now stateless.
+
 ### Deprecated
 
 - `LayoutMode.legacy` is deprecated and will be removed in a future release.