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.

Sign up for GitHub

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

Fix media span and task list memory leaks #1088

Merged
merged 4 commits into from
Jul 25, 2024
Merged

Fix media span and task list memory leaks #1088

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12401d0
Fix leak in media span.
khaykov Jul 25, 2024
5b0653e
Use context ref in task list span.
khaykov Jul 25, 2024
9dc1510
Use setter for media span listeners. Clear task list refresh listener…
khaykov Jul 25, 2024
1d07842
Make drawable null safe in task list span
khaykov Jul 25, 2024
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
22 changes: 15 additions & 7 deletions aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt
View file
Open in desktop
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
12 changes: 9 additions & 3 deletions aztec/src/main/kotlin/org/wordpress/aztec/spans/AztecMediaSpan.kt
View file
Open in desktop
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)
}
}
8 changes: 5 additions & 3 deletions aztec/src/main/kotlin/org/wordpress/aztec/spans/AztecTaskListSpan.kt
View file
Open in desktop
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,7 +101,7 @@ 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)
Expand Down
Loading