From 0b4c6cbcb160f15d4cd8545b52a7fa4103eca8da Mon Sep 17 00:00:00 2001 From: Arthur Milchior Date: Sun, 18 Jun 2023 15:40:44 +0200 Subject: [PATCH] NF: Remove setTitle in AbstractFlashCardViewer It probably should not be `NF`, but here it is. Because each time that `setTitle` is called, `updateActionBar` is called immediately after and change the title anyway. In practice `setTitle` was setting a wrong title in the Reviewer, as it was using current deck instead of the name of the current card. Worse, while moving access to `col` to coroutine, I realized that this created a race condition, where the title changed. That was catched by automated test, so that's great actually. We can also decide that actually, we wanted to keep `Preview` as title all the time, and current behavior is a bug. In which case I suggest we move `updateActionBar` to `Reviewer` and ensure it does not deal with title anymore. --- .../java/com/ichi2/anki/AbstractFlashcardViewer.kt | 4 ---- .../java/com/ichi2/anki/CardTemplatePreviewer.kt | 6 ------ AnkiDroid/src/main/java/com/ichi2/anki/Previewer.kt | 4 ---- AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt | 12 ------------ .../ichi2/anki/AbstractFlashcardViewerCommandTest.kt | 4 ---- .../anki/AbstractFlashcardViewerKeyboardInputTest.kt | 4 ---- .../com/ichi2/anki/AbstractFlashcardViewerTest.kt | 1 - .../java/com/ichi2/anki/ReviewerKeyboardInputTest.kt | 6 ------ 8 files changed, 41 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt index 1ca4b385bcfd..67e9c9b9fa9b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt @@ -568,8 +568,6 @@ abstract class AbstractFlashcardViewer : refreshActionBar() } - protected abstract fun setTitle() - // Finish initializing the activity after the collection has been correctly loaded public override fun onCollectionLoaded(col: Collection) { super.onCollectionLoaded(col) @@ -601,7 +599,6 @@ abstract class AbstractFlashcardViewer : registerExternalStorageListener() restoreCollectionPreferences(col) initLayout() - setTitle() mHtmlGenerator = createInstance(this, typeAnswer!!, mBaseUrl!!) // Initialize text-to-speech. This is an asynchronous operation. @@ -631,7 +628,6 @@ abstract class AbstractFlashcardViewer : } automaticAnswer.enable() // Reset the activity title - setTitle() updateActionBar() selectNavigationItem(-1) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplatePreviewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplatePreviewer.kt index 3d9318d9d90c..5071122b4e44 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplatePreviewer.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplatePreviewer.kt @@ -132,12 +132,6 @@ open class CardTemplatePreviewer : AbstractFlashcardViewer() { closeCardTemplatePreviewer() } - override fun setTitle() { - if (supportActionBar != null) { - supportActionBar?.setTitle(R.string.preview_title) - } - } - override fun initLayout() { super.initLayout() topBarLayout!!.visibility = View.GONE diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Previewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Previewer.kt index 78be50b32944..5dd09e9e0d65 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/Previewer.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/Previewer.kt @@ -144,10 +144,6 @@ class Previewer : AbstractFlashcardViewer() { throw IllegalStateException("newCardList was empty") } - override fun setTitle() { - supportActionBar!!.setTitle(R.string.preview_title) - } - override fun initLayout() { super.initLayout() topBarLayout!!.visibility = View.GONE diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt index d58789bd3760..60f655c8606f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt @@ -315,18 +315,6 @@ open class Reviewer : col.sched.deferReset() } - override fun setTitle() { - val title: String = if (colIsOpen()) { - Decks.basename(col.decks.current().getString("name")) - } else { - Timber.e("Could not set title in reviewer because collection closed") - "" - } - supportActionBar!!.title = title - super.setTitle(title) - supportActionBar!!.subtitle = "" - } - override fun getContentViewAttr(fullscreenMode: FullScreenMode): Int { return when (fullscreenMode) { FullScreenMode.BUTTONS_ONLY -> R.layout.reviewer_fullscreen diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerCommandTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerCommandTest.kt index 08cf7461b5a7..febb46e01d83 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerCommandTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerCommandTest.kt @@ -197,10 +197,6 @@ class AbstractFlashcardViewerCommandTest : RobolectricTest() { currentCardOverride = card } - override fun setTitle() { - // Intentionally blank - } - override fun performReload() { // intentionally blank } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerKeyboardInputTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerKeyboardInputTest.kt index a69ecd815e97..1184d2810885 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerKeyboardInputTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerKeyboardInputTest.kt @@ -100,10 +100,6 @@ class AbstractFlashcardViewerKeyboardInputTest : RobolectricTest() { } } - override fun setTitle() { - // required for interface. Intentionally left blank - } - fun focusTextField() { mFocusTextField = true } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerTest.kt index 35e912599628..c3663000f9f2 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/AbstractFlashcardViewerTest.kt @@ -45,7 +45,6 @@ class AbstractFlashcardViewerTest : RobolectricTest() { class NonAbstractFlashcardViewer : AbstractFlashcardViewer() { var answered: Int? = null private var mLastTime = 0 - override fun setTitle() {} override fun performReload() { // intentionally blank } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt index 745786085942..79e42ab9578b 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt @@ -42,7 +42,6 @@ import org.mockito.ArgumentMatchers import org.mockito.Mockito import org.mockito.kotlin.whenever import timber.log.Timber -import java.lang.Exception @RunWith(AndroidJUnit4::class) class ReviewerKeyboardInputTest : RobolectricTest() { @@ -330,11 +329,6 @@ class ReviewerKeyboardInputTest : RobolectricTest() { whenever(keyEvent.isAltPressed).thenReturn(false) return keyEvent } - - override fun setTitle() { - // required for interface. Intentionally left blank - } - fun focusTextField(): KeyboardInputTestReviewer { mFocusTextField = true return this