From 39bf93ffc081b3a227755ec8df754cc8bb1375ff Mon Sep 17 00:00:00 2001 From: Mario Zorz Date: Thu, 28 Mar 2019 14:06:28 -0300 Subject: [PATCH 1/5] added InputFilter that performs a substation of insertion+AztecImageSpan when an insertion right before an AztecImageSpan is about to happen, to prevent a crash on Android 8.x --- .../kotlin/org/wordpress/aztec/AztecText.kt | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt index 6f7823a4c..e364cd7f8 100644 --- a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt +++ b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt @@ -420,7 +420,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown // triggers ClickableSpan onClick() events movementMethod = EnhancedMovementMethod - setupBackspaceAndEnterDetection() + setupKeyListenersAndInputFilters() //disable auto suggestions/correct for older devices if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) { @@ -440,12 +440,44 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown // Setup the keyListener(s) for Backspace and Enter key. // Backspace: If listener does return false we remove the style here // Enter: Ask the listener if we need to insert or not the char - private fun setupBackspaceAndEnterDetection() { + private fun setupKeyListenersAndInputFilters() { //hardware keyboard setOnKeyListener { _, _, event -> handleBackspaceAndEnter(event) } + // This InputFilter created only for the purpose of avoiding crash described here: + // https://android-review.googlesource.com/c/platform/frameworks/base/+/634929 + // https://github.com/wordpress-mobile/AztecEditor-Android/issues/729 + // the rationale behind this workaround is that the specific crash happens only when adding/deleting text right + // before an AztecImageSpan, so we detect the specific case and re-create the contents only when that happens. + // This is indeed tackling the symptom rather than the actual problem, and should be removed once the real + // problem is fixed at the Android OS level as described in the following url + // https://android-review.googlesource.com/c/platform/frameworks/base/+/634929 + val dynamicLayoutCrashPreventer = InputFilter { source, start, end, dest, dstart, dend -> + var temp = source + if (dest.length > dend+1) { + // if there are any images right after the destination position, hack the text + val spans = dest.getSpans(dstart, dend+1, AztecImageSpan::class.java) + if (spans.isNotEmpty()) { + // test: is this an insertion? + if (dstart == dend) { + // take the source (that is, what is being inserted), and append the Image to it. We will delete + // the original Image later so to not have a duplicate. + // use Spannable to copy / keep the current spans + temp = SpannableStringBuilder(source).append(dest.subSequence(dend, dend+1)) + + // delete the original AztecImageSpan + text.delete(dend, dend+1) + // now insert both the new insertion _and_ the original AztecImageSpan + text.insert(dend, temp) + temp = "" // discard the original source parameter as an ouput from this InputFilter + } + } + } + temp + } + val emptyEditTextBackspaceDetector = InputFilter { source, start, end, dest, dstart, dend -> if (selectionStart == 0 && selectionEnd == 0 && end == 0 && start == 0 @@ -462,7 +494,12 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown source } - filters = arrayOf(emptyEditTextBackspaceDetector) + if (Build.VERSION.SDK_INT == Build.VERSION_CODES.O || Build.VERSION.SDK_INT == Build.VERSION_CODES.O_MR1) { + // dynamicLayoutCrashPreventer needs to be first in array as these are going to be chained when processed + filters = arrayOf(dynamicLayoutCrashPreventer, emptyEditTextBackspaceDetector) + } else { + filters = arrayOf(emptyEditTextBackspaceDetector) + } } private fun handleBackspaceAndEnter(event: KeyEvent): Boolean { From 79e17406544063392986247ebdf129c301cf7b0f Mon Sep 17 00:00:00 2001 From: Mario Zorz Date: Fri, 29 Mar 2019 09:10:52 -0300 Subject: [PATCH 2/5] added disableCrashPreventerInputFilter and disableMediaDeletedListener to prevent inputFilter from being called twice and MediaDeletedListener from being mistakingly called by a non-user operation --- .../kotlin/org/wordpress/aztec/AztecText.kt | 36 ++++++++++++++++++- ...DeleteMediaElementWatcherAPI25AndHigher.kt | 4 +++ .../DeleteMediaElementWatcherPreAPI25.kt | 4 +++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt index e364cd7f8..a8d9c67cd 100644 --- a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt +++ b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt @@ -224,6 +224,8 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown private var consumeSelectionChangedEvent: Boolean = false private var isInlineTextHandlerEnabled: Boolean = true private var bypassObservationQueue: Boolean = false + private var bypassMediaDeletedListener: Boolean = false + private var bypassCrashPreventerInputFilter: Boolean = false var initialEditorContentParsedSHA256: ByteArray = ByteArray(0) @@ -456,22 +458,34 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown // https://android-review.googlesource.com/c/platform/frameworks/base/+/634929 val dynamicLayoutCrashPreventer = InputFilter { source, start, end, dest, dstart, dend -> var temp = source - if (dest.length > dend+1) { + if (dest.length > dend+1 && !bypassCrashPreventerInputFilter) { // if there are any images right after the destination position, hack the text val spans = dest.getSpans(dstart, dend+1, AztecImageSpan::class.java) if (spans.isNotEmpty()) { // test: is this an insertion? if (dstart == dend) { + + // prevent this filter from running twice when `text.insert()` gets called a few lines below + disableCrashPreventerInputFilter() + // disable MediaDeleted listener before operating on content + disableMediaDeletedListener() + // take the source (that is, what is being inserted), and append the Image to it. We will delete // the original Image later so to not have a duplicate. // use Spannable to copy / keep the current spans temp = SpannableStringBuilder(source).append(dest.subSequence(dend, dend+1)) + // delete the original AztecImageSpan text.delete(dend, dend+1) // now insert both the new insertion _and_ the original AztecImageSpan text.insert(dend, temp) temp = "" // discard the original source parameter as an ouput from this InputFilter + + // re-enable MediaDeleted listener + enableMediaDeletedListener() + // re-enable this very filter + enableCrashPreventerInputFilter() } } } @@ -1340,6 +1354,26 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown bypassObservationQueue = false } + fun disableCrashPreventerInputFilter() { + bypassCrashPreventerInputFilter = true + } + + fun enableCrashPreventerInputFilter() { + bypassCrashPreventerInputFilter = false + } + + fun disableMediaDeletedListener() { + bypassMediaDeletedListener = true + } + + fun enableMediaDeletedListener() { + bypassMediaDeletedListener = false + } + + fun isMediaDeletedListenerDisabled(): Boolean { + return bypassMediaDeletedListener + } + fun isTextChangedListenerDisabled(): Boolean { return consumeEditEvent } diff --git a/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherAPI25AndHigher.kt b/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherAPI25AndHigher.kt index d6e2506df..dd1844fa1 100644 --- a/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherAPI25AndHigher.kt +++ b/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherAPI25AndHigher.kt @@ -17,6 +17,10 @@ class DeleteMediaElementWatcherAPI25AndHigher(aztecText: AztecText) : TextWatche return } + if (aztecTextRef.get()?.isMediaDeletedListenerDisabled() ?: true) { + return + } + if (count > 0) { deleted = true diff --git a/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherPreAPI25.kt b/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherPreAPI25.kt index 318a0fc9f..880569278 100644 --- a/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherPreAPI25.kt +++ b/aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherPreAPI25.kt @@ -14,6 +14,10 @@ class DeleteMediaElementWatcherPreAPI25(aztecText: AztecText) : TextWatcher { return } + if (aztecTextRef.get()?.isMediaDeletedListenerDisabled() ?: true) { + return + } + if (count > 0) { aztecTextRef.get()?.text?.getSpans(start, start + count, AztecMediaSpan::class.java) ?.forEach { From 1ff08ac1b02562b0995608121e6e87680abc6561 Mon Sep 17 00:00:00 2001 From: Mario Zorz Date: Fri, 29 Mar 2019 09:17:01 -0300 Subject: [PATCH 3/5] moved condition check up to make sure this is an insertion case before trying to collect image spans in the position next to the insertion to improve performance as suggested in review --- .../kotlin/org/wordpress/aztec/AztecText.kt | 49 +++++++++---------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt index a8d9c67cd..00f60353e 100644 --- a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt +++ b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt @@ -458,35 +458,32 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown // https://android-review.googlesource.com/c/platform/frameworks/base/+/634929 val dynamicLayoutCrashPreventer = InputFilter { source, start, end, dest, dstart, dend -> var temp = source - if (dest.length > dend+1 && !bypassCrashPreventerInputFilter) { + if (!bypassCrashPreventerInputFilter && dstart == dend && dest.length > dend+1) { + // dstart == dend means this is an insertion // if there are any images right after the destination position, hack the text val spans = dest.getSpans(dstart, dend+1, AztecImageSpan::class.java) if (spans.isNotEmpty()) { - // test: is this an insertion? - if (dstart == dend) { - - // prevent this filter from running twice when `text.insert()` gets called a few lines below - disableCrashPreventerInputFilter() - // disable MediaDeleted listener before operating on content - disableMediaDeletedListener() - - // take the source (that is, what is being inserted), and append the Image to it. We will delete - // the original Image later so to not have a duplicate. - // use Spannable to copy / keep the current spans - temp = SpannableStringBuilder(source).append(dest.subSequence(dend, dend+1)) - - - // delete the original AztecImageSpan - text.delete(dend, dend+1) - // now insert both the new insertion _and_ the original AztecImageSpan - text.insert(dend, temp) - temp = "" // discard the original source parameter as an ouput from this InputFilter - - // re-enable MediaDeleted listener - enableMediaDeletedListener() - // re-enable this very filter - enableCrashPreventerInputFilter() - } + // prevent this filter from running twice when `text.insert()` gets called a few lines below + disableCrashPreventerInputFilter() + // disable MediaDeleted listener before operating on content + disableMediaDeletedListener() + + // take the source (that is, what is being inserted), and append the Image to it. We will delete + // the original Image later so to not have a duplicate. + // use Spannable to copy / keep the current spans + temp = SpannableStringBuilder(source).append(dest.subSequence(dend, dend+1)) + + + // delete the original AztecImageSpan + text.delete(dend, dend+1) + // now insert both the new insertion _and_ the original AztecImageSpan + text.insert(dend, temp) + temp = "" // discard the original source parameter as an ouput from this InputFilter + + // re-enable MediaDeleted listener + enableMediaDeletedListener() + // re-enable this very filter + enableCrashPreventerInputFilter() } } temp From 59ff4c085407c5699fdd4c2f3f2e17d40a3bd80e Mon Sep 17 00:00:00 2001 From: Mario Zorz Date: Fri, 29 Mar 2019 09:39:18 -0300 Subject: [PATCH 4/5] initializing var to null as default, to accept original change --- aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt index 00f60353e..3bb9664ce 100644 --- a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt +++ b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt @@ -457,7 +457,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown // problem is fixed at the Android OS level as described in the following url // https://android-review.googlesource.com/c/platform/frameworks/base/+/634929 val dynamicLayoutCrashPreventer = InputFilter { source, start, end, dest, dstart, dend -> - var temp = source + var temp : CharSequence? = null if (!bypassCrashPreventerInputFilter && dstart == dend && dest.length > dend+1) { // dstart == dend means this is an insertion // if there are any images right after the destination position, hack the text From fd3284c3713907eceecef883f49a69b534208ae9 Mon Sep 17 00:00:00 2001 From: Mario Zorz Date: Fri, 29 Mar 2019 09:43:04 -0300 Subject: [PATCH 5/5] fixed lint warning --- aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt index 3bb9664ce..803bc85e7 100644 --- a/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt +++ b/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt @@ -458,7 +458,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown // https://android-review.googlesource.com/c/platform/frameworks/base/+/634929 val dynamicLayoutCrashPreventer = InputFilter { source, start, end, dest, dstart, dend -> var temp : CharSequence? = null - if (!bypassCrashPreventerInputFilter && dstart == dend && dest.length > dend+1) { + if (!bypassCrashPreventerInputFilter && dstart == dend && dest.length > dend+1) { // dstart == dend means this is an insertion // if there are any images right after the destination position, hack the text val spans = dest.getSpans(dstart, dend+1, AztecImageSpan::class.java) @@ -473,7 +473,6 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown // use Spannable to copy / keep the current spans temp = SpannableStringBuilder(source).append(dest.subSequence(dend, dend+1)) - // delete the original AztecImageSpan text.delete(dend, dend+1) // now insert both the new insertion _and_ the original AztecImageSpan