Skip to content

Commit

Permalink
Merge pull request #1088 from wordpress-mobile/issue/fix-media-span-a…
Browse files Browse the repository at this point in the history
…nd-task-list-memory-leaks

Fix media span and task list memory leaks
  • Loading branch information
khaykov authored Jul 25, 2024
2 parents db247eb + 1d07842 commit 2d66175
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 17 deletions.
22 changes: 15 additions & 7 deletions aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown
blockEditorDialog!!.dismiss()
}
EnhancedMovementMethod.setLinkTappedListener(null)
clearTaskListRefreshListeners()
}

// We are exposing this method in order to allow subclasses to set their own alpha value
Expand Down Expand Up @@ -1711,19 +1712,19 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown
val imageSpans = editable.getSpans(start, end, AztecImageSpan::class.java)
imageSpans.forEach {
it.onImageTappedListener = onImageTappedListener
it.onMediaDeletedListener = onMediaDeletedListener
it.setOnMediaDeletedListener(onMediaDeletedListener)
}

val videoSpans = editable.getSpans(start, end, AztecVideoSpan::class.java)
videoSpans.forEach {
it.onVideoTappedListener = onVideoTappedListener
it.onMediaDeletedListener = onMediaDeletedListener
it.setOnMediaDeletedListener(onMediaDeletedListener)
}

val audioSpans = editable.getSpans(start, end, AztecAudioSpan::class.java)
audioSpans.forEach {
it.onAudioTappedListener = onAudioTappedListener
it.onMediaDeletedListener = onMediaDeletedListener
it.setOnMediaDeletedListener(onMediaDeletedListener)
}

val unknownHtmlSpans = editable.getSpans(start, end, UnknownHtmlSpan::class.java)
Expand Down Expand Up @@ -1763,9 +1764,9 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown
taskList.onRefresh = null
this.editableText.removeSpan(taskList)
val newSpan = if (taskList is AztecTaskListSpanAligned) {
AztecTaskListSpanAligned(taskList.nestingLevel, taskList.attributes, taskList.context, taskList.listStyle, taskList.align)
AztecTaskListSpanAligned(taskList.nestingLevel, taskList.attributes, context, taskList.listStyle, taskList.align)
} else {
AztecTaskListSpan(taskList.nestingLevel, taskList.attributes, taskList.context, taskList.listStyle)
AztecTaskListSpan(taskList.nestingLevel, taskList.attributes, context, taskList.listStyle)
}
newSpan.onRefresh = {
refreshTaskListSpan(it)
Expand All @@ -1774,6 +1775,13 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown
setSelection(selStart, selEnd)
}

private fun clearTaskListRefreshListeners() {
val taskLists = this.editableText.getSpans(0, this.editableText.length, AztecTaskListSpan::class.java)
taskLists.forEach { taskList ->
taskList.onRefresh = null
}
}

fun disableTextChangedListener() {
consumeEditEvent = true
}
Expand Down Expand Up @@ -2217,7 +2225,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown
* Use this method to insert a custom AztecMediaSpan at the cursor position
*/
fun insertMediaSpan(span: AztecMediaSpan) {
span.onMediaDeletedListener = onMediaDeletedListener
span.setOnMediaDeletedListener(onMediaDeletedListener)
lineBlockFormatter.insertMediaSpan(shouldAddMediaInline, span)
}

Expand Down Expand Up @@ -2323,7 +2331,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown

text.removeSpan(clickableSpan)
text.removeSpan(mediaSpan)
aztecMediaSpan.onMediaDeletedListener = onMediaDeletedListener
aztecMediaSpan.setOnMediaDeletedListener(onMediaDeletedListener)
lineBlockFormatter.insertMediaSpanOverCurrentChar(aztecMediaSpan, start)
contentChangeWatcher.notifyContentChanged()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,18 @@ import java.lang.ref.WeakReference
import java.util.ArrayList

abstract class AztecMediaSpan(drawable: Drawable?, override var attributes: AztecAttributes = AztecAttributes(),
var onMediaDeletedListener: AztecText.OnMediaDeletedListener? = null,
onMediaDeletedListener: AztecText.OnMediaDeletedListener? = null,
editor: AztecText? = null) : AztecDynamicImageSpan(drawable), IAztecAttributedSpan {
abstract val TAG: String

private val overlays: ArrayList<Pair<Drawable?, Int>> = ArrayList()

private var onMediaDeletedListenerRef = WeakReference(onMediaDeletedListener)

fun setOnMediaDeletedListener(listener: AztecText.OnMediaDeletedListener?) {
onMediaDeletedListenerRef = WeakReference(listener)
}

init {
textView = editor?.let { WeakReference(editor) }
}
Expand Down Expand Up @@ -96,9 +102,9 @@ abstract class AztecMediaSpan(drawable: Drawable?, override var attributes: Azte
abstract fun onClick()

fun onMediaDeleted() {
onMediaDeletedListener?.onMediaDeleted(attributes)
onMediaDeletedListenerRef.get()?.onMediaDeleted(attributes)
}
fun beforeMediaDeleted() {
onMediaDeletedListener?.beforeMediaDeleted(attributes)
onMediaDeletedListenerRef.get()?.beforeMediaDeleted(attributes)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import org.wordpress.aztec.ITextFormat
import org.wordpress.aztec.R
import org.wordpress.aztec.formatting.BlockFormatter
import org.wordpress.aztec.setTaskList
import java.lang.ref.WeakReference

fun createTaskListSpan(
nestingLevel: Int,
Expand Down Expand Up @@ -61,11 +62,12 @@ class AztecTaskListSpanAligned(
open class AztecTaskListSpan(
override var nestingLevel: Int,
override var attributes: AztecAttributes = AztecAttributes(),
val context: Context,
context: Context,
var listStyle: BlockFormatter.ListStyle = BlockFormatter.ListStyle(0, 0, 0, 0, 0),
var onRefresh: ((AztecTaskListSpan) -> Unit)? = null
) : AztecListSpan(nestingLevel, listStyle.verticalPadding) {
private var toggled: Boolean = false
private var contextRef: WeakReference<Context> = WeakReference(context)
override val TAG = "ul"

override val startTag: String
Expand All @@ -82,7 +84,7 @@ open class AztecTaskListSpan(
top: Int, baseline: Int, bottom: Int,
text: CharSequence, start: Int, end: Int,
first: Boolean, l: Layout) {
if (!first) return
if (!first || contextRef.get() == null) return

val spanStart = (text as Spanned).getSpanStart(this)
val spanEnd = text.getSpanEnd(this)
Expand All @@ -99,24 +101,24 @@ open class AztecTaskListSpan(
val drawableHeight = (0.8 * (p.fontMetrics.bottom - p.fontMetrics.top))
// Make sure the marker is correctly aligned on RTL languages
val markerStartPosition: Float = x + (listStyle.indicatorMargin * dir) * 1f
val d: Drawable = context.resources.getDrawable(R.drawable.ic_checkbox, null)
val d: Drawable? = contextRef.get()?.resources?.getDrawable(R.drawable.ic_checkbox, null)
val leftBound = markerStartPosition.toInt()
if (isChecked(text, lineIndex)) {
d.state = intArrayOf(android.R.attr.state_checked)
d?.state = intArrayOf(android.R.attr.state_checked)
} else {
d.state = intArrayOf()
d?.state = intArrayOf()
}
val (startShift, endShift) = if (dir > 0) {
0.8 to 0.2
} else {
0.2 to 0.8
}

d.setBounds((leftBound - drawableHeight * startShift).toInt().coerceAtLeast(0),
d?.setBounds((leftBound - drawableHeight * startShift).toInt().coerceAtLeast(0),
(baseline - drawableHeight * 0.8).toInt(),
(leftBound + drawableHeight * endShift).toInt(),
(baseline + drawableHeight * 0.2).toInt())
d.draw(c)
d?.draw(c)

p.color = oldColor
p.style = style
Expand Down

0 comments on commit 2d66175

Please sign in to comment.