From e9066b3d52cff0be51eb68cb570797f2ca098a24 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Wed, 5 Feb 2025 14:00:41 +0700 Subject: [PATCH 1/3] change(ios): relocates the "enter foreground" kbd-reload trigger Addresses part of #12216 --- .../Classes/Keyboard/InputViewController.swift | 2 +- .../Classes/Keyboard/KeymanWebViewController.swift | 8 -------- .../Classes/MainViewController/MainViewController.swift | 8 ++++++++ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 16acd10f9e..98f6290276 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -589,7 +589,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { } // KeymanWebViewController maintenance methods - func reload() { + public func reload() { keymanWeb.reloadKeyboard() } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift index a0c8c6d5e4..72822bbbfc 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift @@ -72,14 +72,6 @@ class KeymanWebViewController: UIViewController { super.init(nibName: nil, bundle: nil) _ = view - - NotificationCenter.default.addObserver( - forName: UIApplication.willEnterForegroundNotification, - object: nil, - queue: OperationQueue.main - ) { _ in - self.reloadKeyboard() - } } required init?(coder aDecoder: NSCoder) { diff --git a/ios/keyman/Keyman/Keyman/Classes/MainViewController/MainViewController.swift b/ios/keyman/Keyman/Keyman/Classes/MainViewController/MainViewController.swift index 35ee78c5db..de6f00da63 100644 --- a/ios/keyman/Keyman/Keyman/Classes/MainViewController/MainViewController.swift +++ b/ios/keyman/Keyman/Keyman/Classes/MainViewController/MainViewController.swift @@ -112,6 +112,14 @@ class MainViewController: UIViewController, TextViewDelegate, UIActionSheetDeleg forName: Notifications.keyboardRemoved, observer: self, function: MainViewController.keyboardRemoved) + + NotificationCenter.default.addObserver( + forName: UIApplication.willEnterForegroundNotification, + object: nil, + queue: OperationQueue.main + ) { _ in + Manager.shared.inputViewController.reload() + } // Unfortunately, it's the main app with the file definitions. // We have to gerry-rig this so that the framework-based SettingsViewController From b8aaab6ea21caafc8cb4784664c93ca6280f737c Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Wed, 5 Feb 2025 14:01:18 +0700 Subject: [PATCH 2/3] change(ios): relocates keyboard hide+show notification handling Addresses part of #12216 --- .../Keyboard/InputViewController.swift | 10 +++++++- .../Keyboard/KeymanWebViewController.swift | 20 --------------- .../KMEI/KeymanEngine/Classes/Manager.swift | 25 +++++++++++++++++++ 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 98f6290276..87794aac3b 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -206,7 +206,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { keymanWeb = KeymanWebViewController(storage: Storage.active) super.init(nibName: nibNameOrNil, bundle: nibBundleOrNil) - var message = self.hasFullAccess ? "hasFullAccess: true" : "hasFullAccess: false" + let message = self.hasFullAccess ? "hasFullAccess: true" : "hasFullAccess: false" os_log("%{public}s", log: KeymanEngineLogger.settings, type: .default, message) SentryManager.breadcrumb(message) @@ -623,10 +623,18 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { func showHelpBubble() { keymanWeb.showHelpBubble() } + + func dismissHelpBubble() { + keymanWeb.dismissHelpBubble() + } func showHelpBubble(afterDelay delay: TimeInterval) { keymanWeb.showHelpBubble(afterDelay: delay) } + + internal func enforceKeyboardSize() { + keymanWeb.resizeKeyboard() + } func clearText() { setContextState(text: nil, range: NSRange(location: 0, length: 0)) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift index 72822bbbfc..c0e7cefd0c 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift @@ -146,11 +146,6 @@ class KeymanWebViewController: UIViewController { view = webView - NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillShow), - name: UIResponder.keyboardWillShowNotification, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillHide), - name: UIResponder.keyboardWillHideNotification, object: nil) - reloadKeyboard() } @@ -982,18 +977,3 @@ extension KeymanWebViewController { return keyboardMenuView != nil } } - -// MARK: - Keyboard Notifications -extension KeymanWebViewController { - @objc func keyboardWillShow(_ notification: Notification) { - resizeKeyboard() - - if Manager.shared.isKeymanHelpOn { - showHelpBubble(afterDelay: 1.5) - } - } - - @objc func keyboardWillHide(_ notification: Notification) { - dismissHelpBubble() - } -} diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift index 4ab0da660b..25b1f143cb 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift @@ -239,10 +239,35 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { SentryManager.capture(error, message:message) } } + + // Keep these within our singleton Manager instance. As we never remove these, + // setting them within something prone to replacement (like the keyboard when in + // app-extension mode) can lead to memory leaks. See #12216. + NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillShow), // + name: UIResponder.keyboardWillShowNotification, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillHide), // + name: UIResponder.keyboardWillHideNotification, object: nil) // We used to preload the old KeymanWebViewController, but now that it's embedded within the // InputViewController, that's not exactly viable. } + + // MARK: - Keyboard Notifications + // Do NOT place these anywhere within InputViewController or its children. + // So far as we can tell, the OS causes the app extension to sometimes throw + // instances away, and maintaining references via notification to them can + // cause memory leaks. Refer to #12216. + @objc func keyboardWillShow(_ notification: Notification) { + inputViewController?.enforceKeyboardSize() + + if Manager.shared.isKeymanHelpOn { + inputViewController?.showHelpBubble(afterDelay: 1.5) + } + } + + @objc func keyboardWillHide(_ notification: Notification) { + inputViewController?.dismissHelpBubble() + } // MARK: - Keyboard management From 6cad46fcf01db149655bd64c3e18791764b70952 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Fri, 7 Feb 2025 15:19:54 +0700 Subject: [PATCH 3/3] fix(ios): make KeyboardMenuView ref to ancestor `weak` --- .../Keyboard/InputViewController.swift | 24 +++++++++++++++---- .../Classes/Keyboard/KeyboardMenuView.swift | 4 +++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 87794aac3b..7923e3eb93 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -165,6 +165,8 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { // For now, should be mostly upon keymanWeb.view.heightAnchor. var portraitConstraint: NSLayoutConstraint? var landscapeConstraint: NSLayoutConstraint? + + var outerWidthConstraint: NSLayoutConstraint? private var keymanWeb: KeymanWebViewController private var swallowBackspaceTextChange: Bool = false @@ -300,6 +302,16 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { keymanWeb.shouldReload = true } } + + open override func viewDidDisappear(_ animated: Bool) { + super.viewDidDisappear(animated) + + if outerWidthConstraint != nil { + outerWidthConstraint?.isActive = false + self.inputView?.removeConstraint(self.outerWidthConstraint!) + outerWidthConstraint = nil + } + } open override func textDidChange(_ textInput: UITextInput?) { // Swallows self-triggered calls from emptying the context due to keyboard rules @@ -527,11 +539,15 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { } // These require the view to appear - parent and our relationship with it must exist! + // ... wait, is THIS possibly a leak source? private func setOuterConstraints() { - var baseWidthConstraint: NSLayoutConstraint - baseWidthConstraint = self.inputView!.widthAnchor.constraint(equalTo: parent!.view.safeAreaLayoutGuide.widthAnchor) - baseWidthConstraint.priority = UILayoutPriority(rawValue: 999) - baseWidthConstraint.isActive = true + guard outerWidthConstraint == nil else { + outerWidthConstraint!.isActive = true + return + } + outerWidthConstraint = self.inputView!.widthAnchor.constraint(equalTo: parent!.view.safeAreaLayoutGuide.widthAnchor) + outerWidthConstraint!.priority = UILayoutPriority(rawValue: 999) + outerWidthConstraint!.isActive = true } public var kmwHeight: CGFloat { diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift index 397697222c..1823b143b3 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift @@ -26,7 +26,9 @@ class KeyboardMenuView: UIView, UITableViewDelegate, UITableViewDataSource, UIGe private var tableView: UITableView? private let closeButtonTitle: String? - private var _inputViewController: InputViewController? + // Is populated during init from a weak ref by KeymanWebViewController, + // and instances of this class are owned by the same. + private weak var _inputViewController: InputViewController? override var inputViewController: InputViewController? { return _inputViewController }