Merge pull request #620 from wordpress-mobile/issue/618-onimagedeleted-called-in-sequence

Issue/618 onMediaDeleted called unnecessarily in sequence

Author: 0nko

aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt

@@ -88,7 +88,8 @@ import org.wordpress.aztec.util.AztecLog
 import org.wordpress.aztec.util.SpanWrapper
 import org.wordpress.aztec.util.coerceToHtmlText
 import org.wordpress.aztec.watchers.BlockElementWatcher
-import org.wordpress.aztec.watchers.DeleteMediaElementWatcher
+import org.wordpress.aztec.watchers.DeleteMediaElementWatcherAPI25AndHigher
+import org.wordpress.aztec.watchers.DeleteMediaElementWatcherPreAPI25
 import org.wordpress.aztec.watchers.EndOfBufferMarkerAdder
 import org.wordpress.aztec.watchers.EndOfParagraphMarkerAdder
 import org.wordpress.aztec.watchers.FullWidthImageElementWatcher
@@ -445,7 +446,11 @@ class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknownHtmlT
         EndOfBufferMarkerAdder.install(this)
         ZeroIndexContentWatcher.install(this)
 
-        DeleteMediaElementWatcher.install(this)
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N_MR1) {
+            DeleteMediaElementWatcherAPI25AndHigher.install(this)
+        } else {
+            DeleteMediaElementWatcherPreAPI25.install(this)
+        }
 
         // History related logging has to happen before the changes in [ParagraphCollapseRemover]
         addHistoryLoggingWatcher()
@@ -852,6 +857,13 @@ class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknownHtmlT
         return (observationQueue.hasActiveBuckets() && !bypassObservationQueue && (watchersNestingLevel == 1))
     }
 
+    fun isObservationQueueBeingPopulated() : Boolean {
+        // TODO: use the value that is going to be published from ObservationQueue.MAXIMUM_TIME_BETWEEN_EVENTS_IN_PATTERN_MS
+        val MAXIMUM_TIME_BETWEEN_EVENTS_IN_PATTERN_MS = 100
+        return !observationQueue.isEmpty() &&
+                ((System.currentTimeMillis() - observationQueue.last().timestamp) < MAXIMUM_TIME_BETWEEN_EVENTS_IN_PATTERN_MS)
+    }
+
     override fun beforeTextChanged(text: CharSequence, start: Int, count: Int, after: Int) {
         addWatcherNestingLevel()
         if (!isViewInitialized) return

aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherAPI25AndHigher.kt

@@ -0,0 +1,61 @@
+package org.wordpress.aztec.watchers
+
+import android.text.Editable
+import android.text.TextWatcher
+import org.wordpress.aztec.AztecText
+import org.wordpress.aztec.spans.AztecMediaSpan
+import java.lang.ref.WeakReference
+
+class DeleteMediaElementWatcherAPI25AndHigher(aztecText: AztecText) : TextWatcher {
+    private val aztecTextRef: WeakReference<AztecText?> = WeakReference(aztecText)
+    private var deleted = false
+    private var queueHasBeenPopulatedInThisTimeframe = false
+    private var deletedSpans = ArrayList<AztecMediaSpan>()
+
+    override fun beforeTextChanged(text: CharSequence, start: Int, count: Int, after: Int) {
+        if (aztecTextRef.get()?.isTextChangedListenerDisabled() ?: true) {
+            return
+        }
+
+        if (count > 0) {
+            deleted = true
+
+            // we need to save the spans for later reference, as these spans are going to not exist anymore by the time
+            // the afterTextChanged event arrives. So, adding them to a local deletedSpans array.
+            aztecTextRef.get()?.text?.getSpans(start, start + count, AztecMediaSpan::class.java)
+                    ?.forEach {
+                        deletedSpans.add(it)
+                    }
+
+            // only call the onMediaDeleted callback if we are sure the ObservationQueue has not been filled with
+            // platform-only events in a short time. These platform-originated events shall not be confused with
+            // real user deletions.
+            aztecTextRef.get()?.postDelayed( object : Runnable {
+                override fun run() {
+                    if (!queueHasBeenPopulatedInThisTimeframe) {
+                        deletedSpans.forEach { it.onMediaDeleted() }
+                    }
+                    // reset flag
+                    deletedSpans.clear()
+                    queueHasBeenPopulatedInThisTimeframe = false
+                }
+            }, 500)
+        }
+    }
+
+    override fun onTextChanged(text: CharSequence, start: Int, before: Int, count: Int) {
+        // no op
+    }
+
+    override fun afterTextChanged(text: Editable) {
+        if (deleted && aztecTextRef.get()?.isObservationQueueBeingPopulated() ?: true) {
+            queueHasBeenPopulatedInThisTimeframe = true
+        }
+    }
+
+    companion object {
+        fun install(text: AztecText) {
+            text.addTextChangedListener(DeleteMediaElementWatcherAPI25AndHigher(text))
+        }
+    }
+}

…ec/watchers/DeleteMediaElementWatcher.kt …ers/DeleteMediaElementWatcherPreAPI25.ktaztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcher.kt renamed to aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherPreAPI25.kt aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcher.kt renamed to aztec/src/main/kotlin/org/wordpress/aztec/watchers/DeleteMediaElementWatcherPreAPI25.kt

@@ -6,7 +6,7 @@ import org.wordpress.aztec.AztecText
 import org.wordpress.aztec.spans.AztecMediaSpan
 import java.lang.ref.WeakReference
 
-class DeleteMediaElementWatcher(aztecText: AztecText) : TextWatcher {
+class DeleteMediaElementWatcherPreAPI25(aztecText: AztecText) : TextWatcher {
     private val aztecTextRef: WeakReference<AztecText?> = WeakReference(aztecText)
 
     override fun beforeTextChanged(text: CharSequence, start: Int, count: Int, after: Int) {
@@ -32,7 +32,7 @@ class DeleteMediaElementWatcher(aztecText: AztecText) : TextWatcher {
 
     companion object {
         fun install(text: AztecText) {
-            text.addTextChangedListener(DeleteMediaElementWatcher(text))
+            text.addTextChangedListener(DeleteMediaElementWatcherPreAPI25(text))
         }
     }
-}
+}