From fc2a17890c990bfe7241daf547e02fadded4307a Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 25 Feb 2021 14:43:35 +0700 Subject: [PATCH] docs(common/models): adds comments about bug in iOS insertText --- .../Classes/InputViewController.swift | 168 ++++++++++-------- 1 file changed, 89 insertions(+), 79 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/InputViewController.swift index 6742c7cdc7..cd4092140f 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/InputViewController.swift @@ -407,94 +407,104 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { textDocumentProxy.insertText(newText) } - // Should never change throughout the next loop; right-deletions only! - let oldLeftContext = textDocumentProxy.documentContextBeforeInput ?? "" - var pointsToDelete = numCharsToRightDelete - while pointsToDelete > 0 { - let oldContext = textDocumentProxy.documentContextAfterInput ?? "" - self.swallowContextChangeCount += 1 - // Asynchronously triggers a context change. - textDocumentProxy.adjustTextPosition(byCharacterOffset: 1) - let newContext = textDocumentProxy.documentContextAfterInput ?? "" + // NOTE: when a script uses character clusters, the code below fails to + // produce the correct effects despite `textDocumentProxy` reporting that + // the code correctly manipulates the text. There's either a bug in Apple's + // code or something critical that we're missing here. + // + // The `textDidChange` call that occurs after this `insertText` will + // report the final, 'incorrect' details, rather than what is reported + // at the end of this method. + if numCharsToRightDelete > 0 { + // Should never change throughout the next loop; right-deletions only! + let oldLeftContext = textDocumentProxy.documentContextBeforeInput ?? "" + var pointsToDelete = numCharsToRightDelete + while pointsToDelete > 0 { + let oldContext = textDocumentProxy.documentContextAfterInput ?? "" + self.swallowContextChangeCount += 1 + // Asynchronously triggers a context change. + textDocumentProxy.adjustTextPosition(byCharacterOffset: 1) + let newContext = textDocumentProxy.documentContextAfterInput ?? "" - var failsafeCount = 0 - while (textDocumentProxy.documentContextBeforeInput ?? "" != oldLeftContext) - && failsafeCount < numCharsToRightDelete { // mild breakage > locked kbd - // While adjustTextPosition makes cluster-based jumps... - // deleteBackward does not. So, we 'force' it. - textDocumentProxy.deleteBackward() - failsafeCount += 1 - } + var failsafeCount = 0 + while (textDocumentProxy.documentContextBeforeInput ?? "" != oldLeftContext) + && failsafeCount < numCharsToRightDelete { // mild breakage > locked kbd + // While adjustTextPosition makes cluster-based jumps... + // deleteBackward does not. So, we 'force' it. + textDocumentProxy.deleteBackward() + failsafeCount += 1 + } - // Determine the removed codepoint count. Also, find the index - // of the first character that shouldn't be removed, if possible. - let unitsDeleted = oldContext.utf16.count - newContext.utf16.count - var pointsDeleted = 0 + // Determine the removed codepoint count. Also, find the index + // of the first character that shouldn't be removed, if possible. + let unitsDeleted = oldContext.utf16.count - newContext.utf16.count + var pointsDeleted = 0 - // Our current index within the deleted context. - var deletedIndex = oldContext.utf16.startIndex - // Marks the first index we wish to NOT remove. - var remnantIndex: String.Index? = nil - // Marks the first index that was NOT removed by deleteBackward. - let undeletedStartIndex = oldContext.utf16.index(deletedIndex, offsetBy: unitsDeleted) + // Our current index within the deleted context. + var deletedIndex = oldContext.utf16.startIndex + // Marks the first index we wish to NOT remove. + var remnantIndex: String.Index? = nil + // Marks the first index that was NOT removed by deleteBackward. + let undeletedStartIndex = oldContext.utf16.index(deletedIndex, offsetBy: unitsDeleted) - while deletedIndex < undeletedStartIndex { - if(InputViewController.isSurrogate(oldContext.utf16[deletedIndex])) { - // Check - is it truly a surrogate pair? - let pairedIndex = oldContext.utf16.index(after: deletedIndex) - if(InputViewController.isSurrogate(oldContext.utf16[pairedIndex])) { - // If so, pre-emptively increase the index - the pair will count - // as a single character as a result. - deletedIndex = pairedIndex + while deletedIndex < undeletedStartIndex { + if(InputViewController.isSurrogate(oldContext.utf16[deletedIndex])) { + // Check - is it truly a surrogate pair? + let pairedIndex = oldContext.utf16.index(after: deletedIndex) + if(InputViewController.isSurrogate(oldContext.utf16[pairedIndex])) { + // If so, pre-emptively increase the index - the pair will count + // as a single character as a result. + deletedIndex = pairedIndex + } + } + pointsDeleted += 1 + deletedIndex = oldContext.utf16.index(after: deletedIndex) + + // Intended end of deletion found! + if pointsDeleted == pointsToDelete { + remnantIndex = deletedIndex + break // There's nothing to be gained by further loop iterations. } } - pointsDeleted += 1 - deletedIndex = oldContext.utf16.index(after: deletedIndex) - // Intended end of deletion found! - if pointsDeleted == pointsToDelete { - remnantIndex = deletedIndex - break // There's nothing to be gained by further loop iterations. + if let remnantIndex = remnantIndex, remnantIndex < undeletedStartIndex { + // We need to restore some of the deleted text! + let remnant = String(oldContext.utf16[remnantIndex.. oldLeftContext.count) { + textDocumentProxy.adjustTextPosition(byCharacterOffset: -remnant.count) + leftContext = textDocumentProxy.documentContextBeforeInput ?? "" + } + + // FAILSAFE, non-ideal behavior below. + // + // It's probably (?) better to keep the caret at the end of a cluster that + // previously started before the caret, even if right-deletions add new + // characters to the cluster. I think. + // + // Khmer example: if right-deletes leave a joeung-S that can attach + // to a main consonant to the left of my cursor, it will 'snap' into + // a cluster. It'd be more natural to have that cluster on the left, + // so I can add more chars to the cluster if desired. + // + // It's also easier to immediately backspace the extra chars if needed + // this way, rather than forcing a caret reposition. + if(leftContext.count < oldLeftContext.count) { + log.debug("Could not reposition caret perfectly after a right-delete!") + textDocumentProxy.adjustTextPosition(byCharacterOffset: 1) + } } + + // We've handled a set of code points; make sure we mark our progress! + pointsToDelete -= pointsDeleted } - - if let remnantIndex = remnantIndex, remnantIndex < undeletedStartIndex { - // We need to restore some of the deleted text! - let remnant = String(oldContext.utf16[remnantIndex.. oldLeftContext.count) { - textDocumentProxy.adjustTextPosition(byCharacterOffset: -remnant.count) - leftContext = textDocumentProxy.documentContextBeforeInput ?? "" - } - - // FAILSAFE, non-ideal behavior below. - // - // It's probably (?) better to keep the caret at the end of a cluster that - // previously started before the caret, even if right-deletions add new - // characters to the cluster. I think. - // - // Khmer example: if right-deletes leave a joeung-S that can attach - // to a main consonant to the left of my cursor, it will 'snap' into - // a cluster. It'd be more natural to have that cluster on the left, - // so I can add more chars to the cluster if desired. - // - // It's also easier to immediately backspace the extra chars if needed - // this way, rather than forcing a caret reposition. - if(leftContext.count < oldLeftContext.count) { - log.debug("Could not reposition caret perfectly after a right-delete!") - textDocumentProxy.adjustTextPosition(byCharacterOffset: 1) - } - } - - // We've handled a set of code points; make sure we mark our progress! - pointsToDelete -= pointsDeleted } }