Skip to content
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

Android 8.x dymanic layout crash preventer #801

Merged
merged 5 commits into from
Apr 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 70 additions & 3 deletions aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -420,7 +422,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) {
Expand All @@ -440,12 +442,52 @@ 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 : 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
val spans = dest.getSpans(dstart, dend+1, AztecImageSpan::class.java)
if (spans.isNotEmpty()) {
// 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
}

val emptyEditTextBackspaceDetector = InputFilter { source, start, end, dest, dstart, dend ->
if (selectionStart == 0 && selectionEnd == 0
&& end == 0 && start == 0
Expand All @@ -462,7 +504,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 {
Expand Down Expand Up @@ -1303,6 +1350,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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class DeleteMediaElementWatcherAPI25AndHigher(aztecText: AztecText) : TextWatche
return
}

if (aztecTextRef.get()?.isMediaDeletedListenerDisabled() ?: true) {
return
}

if (count > 0) {
deleted = true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down