Skip to content

fix(reviewers): support localStorage again #18369

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

Merged
merged 5 commits into from
Jun 10, 2025
Merged

fix(reviewers): support localStorage again #18369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
643deba
fix(reviewers): support localStorage again
BrayanDSO May 25, 2025
a8fdba2
chore: remove unused class
BrayanDSO Jun 4, 2025
79f22de
refactor: add intPref util
BrayanDSO Jun 4, 2025
bd7aab4
feat(reviewers): use obscure fixed port with setting
BrayanDSO Jun 10, 2025
a737c05
feat: move fixed port setting to `Advanced`
BrayanDSO Jun 10, 2025
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
8 changes: 7 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ import com.ichi2.anki.settings.Prefs
import com.ichi2.anki.snackbar.BaseSnackbarBuilderProvider
import com.ichi2.anki.snackbar.SnackbarBuilder
import com.ichi2.anki.snackbar.showSnackbar
import com.ichi2.anki.ui.windows.reviewer.ReviewerFragment
import com.ichi2.anki.utils.OnlyOnce.Method.ANSWER_CARD
import com.ichi2.anki.utils.OnlyOnce.preventSimultaneousExecutions
import com.ichi2.anki.utils.ext.showDialogFragment
Expand Down Expand Up @@ -269,7 +270,7 @@ abstract class AbstractFlashcardViewer :

// needs to be lateinit due to a reliance on Context

val server = AnkiServer(this).also { it.start() }
lateinit var server: AnkiServer

@get:VisibleForTesting
var cardContent: String? = null
Expand Down Expand Up @@ -548,6 +549,8 @@ abstract class AbstractFlashcardViewer :

setContentView(getContentViewAttr(fullscreenMode))

val port = ReviewerFragment.getServerPort()
server = AnkiServer(this, port).also { it.start() }
// Make ACTION_PROCESS_TEXT for in-app searching possible on > Android 4.0
delegate.isHandleNativeActionModesEnabled = true
val mainView = findViewById<View>(android.R.id.content)
Expand Down Expand Up @@ -642,6 +645,9 @@ abstract class AbstractFlashcardViewer :

override fun onDestroy() {
super.onDestroy()
if (this::server.isInitialized) {
server.stop()
}
tts.releaseTts(this)
// WebView.destroy() should be called after the end of use
// http://developer.android.com/reference/android/webkit/WebView.html#destroy()
Expand Down
5 changes: 0 additions & 5 deletions AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,6 @@ open class Reviewer :
}
}

override fun onDestroy() {
super.onDestroy()
server.stop()
}

protected val flagToDisplay: Flag
get() {
return FlagToDisplay(
Expand Down
1 change: 1 addition & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/analytics/UsageAnalytics.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ object UsageAnalytics {
R.string.type_in_answer_focus_key, // Focus ‘type in answer’
R.string.media_import_allow_all_files_key, // Allow all files in media imports
R.string.enable_api_key, // Enable AnkiDroid API
R.string.use_fixed_port_pref_key, // localStorage in Study Screen
// ******************************** App bar buttons ****************************************
R.string.reset_custom_buttons_key,
R.string.custom_button_undo_key,
Expand Down
3 changes: 2 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/pages/AnkiServer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import java.io.ByteArrayInputStream

open class AnkiServer(
private val postHandler: PostRequestHandler,
) : NanoHTTPD(LOCALHOST, 0) {
port: Int = 0,
) : NanoHTTPD(LOCALHOST, port) {
fun baseUrl(): String = "http://$LOCALHOST:$listeningPort/"

// it's faster to serve local files without GZip. see 'page render' in logs
Expand Down
3 changes: 2 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/previewer/CardViewerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ abstract class CardViewerViewModel(

@CallSuper
override fun onCleared() {
super.onCleared()
cardMediaPlayer.close()
server.stop()
super.onCleared()
}

/* *********************************************************************************************
Expand Down
45 changes: 29 additions & 16 deletions AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ object Prefs {
defValue: Int,
): Int = sharedPrefs.getInt(key(keyResId), defValue)

@VisibleForTesting
fun putInt(
@StringRes keyResId: Int,
defValue: Int,
) = sharedPrefs.edit { putInt(key(keyResId), defValue) }

@VisibleForTesting
fun <E> getEnum(
@StringRes keyResId: Int,
Expand Down Expand Up @@ -130,23 +136,24 @@ object Prefs {
}

@VisibleForTesting
class StringPref(
@StringRes private val keyResId: Int,
private val defaultValue: String? = null,
) : ReadWriteProperty<Any?, String?> {
override fun getValue(
thisRef: Any?,
property: KProperty<*>,
): String? = getString(keyResId, defaultValue) ?: defaultValue

override fun setValue(
thisRef: Any?,
property: KProperty<*>,
value: String?,
) {
putString(keyResId, value)
fun intPref(
@StringRes keyResId: Int,
defaultValue: Int,
): ReadWriteProperty<Any, Int> =
object : ReadWriteProperty<Any?, Int> {
override fun getValue(
thisRef: Any?,
property: KProperty<*>,
): Int = getInt(keyResId, defaultValue)

override fun setValue(
thisRef: Any?,
property: KProperty<*>,
value: Int,
) {
putInt(keyResId, value)
}
}
}

// ****************************************************************************************** //
// **************************************** Settings **************************************** //
Expand Down Expand Up @@ -181,6 +188,12 @@ object Prefs {
val answerButtonsSize: Int
get() = getInt(R.string.answer_button_size_preference, 100)

// **************************************** Advanced **************************************** //

var useFixedPortInReviewer by booleanPref(R.string.use_fixed_port_pref_key, false)

var reviewerPort by intPref(R.string.reviewer_port_pref_key, defaultValue = 0)

// ************************************* Developer options ********************************** //

/**
Expand Down
24 changes: 23 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ import com.ichi2.utils.dp
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import timber.log.Timber
import java.net.BindException
import java.net.ServerSocket

class ReviewerFragment :
CardViewerFragment(R.layout.reviewer2),
Expand All @@ -109,7 +112,8 @@ class ReviewerFragment :
DispatchKeyEventListener,
TagsDialogListener {
override val viewModel: ReviewerViewModel by viewModels {
ReviewerViewModel.factory(CardMediaPlayer(), BindingMap(sharedPrefs(), ViewerAction.entries))
val bindingMap = BindingMap(sharedPrefs(), ViewerAction.entries)
ReviewerViewModel.factory(CardMediaPlayer(), bindingMap, getServerPort())
}

override val webView: WebView
Expand Down Expand Up @@ -605,5 +609,23 @@ class ReviewerFragment :

companion object {
fun getIntent(context: Context): Intent = CardViewerActivity.getIntent(context, ReviewerFragment::class)

fun getServerPort(): Int {
if (!Prefs.useFixedPortInReviewer) return 0
return try {
ServerSocket(Prefs.reviewerPort)
.use {
it.reuseAddress = true
it.localPort
}.also {
if (Prefs.reviewerPort == 0) {
Prefs.reviewerPort = it
}
}
} catch (_: BindException) {
Timber.w("Fixed port %d under use. Using dynamic port", Prefs.reviewerPort)
0
}
}
}
}
6 changes: 4 additions & 2 deletions AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ import timber.log.Timber
class ReviewerViewModel(
cardMediaPlayer: CardMediaPlayer,
private val bindingMap: BindingMap<ReviewerBinding, ViewerAction>,
serverPort: Int = 0,
) : CardViewerViewModel(cardMediaPlayer),
ChangeManager.Subscriber,
BindingProcessor<ReviewerBinding, ViewerAction> {
Expand All @@ -103,7 +104,7 @@ class ReviewerViewModel(
val editNoteTagsFlow = MutableSharedFlow<NoteId>()
val setDueDateFlow = MutableSharedFlow<CardId>()

override val server = AnkiServer(this).also { it.start() }
override val server: AnkiServer = AnkiServer(this, serverPort).also { it.start() }
private val stateMutationKey = TimeManager.time.intTimeMS().toString()
val statesMutationEval = MutableSharedFlow<String>()

Expand Down Expand Up @@ -670,10 +671,11 @@ class ReviewerViewModel(
fun factory(
soundPlayer: CardMediaPlayer,
bindingMap: BindingMap<ReviewerBinding, ViewerAction>,
serverPort: Int,
): ViewModelProvider.Factory =
viewModelFactory {
initializer {
ReviewerViewModel(soundPlayer, bindingMap)
ReviewerViewModel(soundPlayer, bindingMap, serverPort)
}
}

Expand Down
3 changes: 3 additions & 0 deletions AnkiDroid/src/main/res/values/10-preferences.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ this formatter is used if the bind only applies to the answer">A: %s</string>
<string name="pref_cat_experimental" maxLength="41" tools:ignore="UnusedResources">Experimental</string>
<string name="readtext_deprecation_warn">AnkiDroid has introduced a better TTS mechanism which is compatible with other Anki clients and includes more voices and improvements to language playback!\n\nPlease upgrade as soon as possible, as this setting will be removed soon</string>
<string name="readtext_reviewer_warn" tools:ignore="UnusedResources">Please upgrade to the new text to speech format</string>
<string name="pref_fixed_port_title" maxLength="41">localStorage in Study Screen</string>
<string name="pref_fixed_port_summary"
>Enables JavaScript localStorage by using a fixed server port in the Study Screen</string>
Copy link
Member

@david-allison david-allison Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our translators aren't gonna like this one

Copy link
Member Author

@BrayanDSO BrayanDSO Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Completely open to simplify it, but I found hard doing that without losing context enough to explain the setting.

Honestly, I tend to prefer just making it the default. Even if you get access to the port,

  1. CORS is rejected by default
  2. All GET requests to the server are rejected

    Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/pages/AnkiServer.kt

    Lines 49 to 55 in 3ae9b9d

    Method.GET -> {
    Timber.d("Rejecting GET request to server %s", session.uri)
    newFixedLengthResponse(Response.Status.NOT_FOUND, null, null)
    }
    else -> {
    Timber.d("Ignored request of unhandled method %s, uri %s", session.method, session.uri)
    newFixedLengthResponse(null)
  3. The POST requests are somewhat restricted (idk about the JS API)

    Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt

    Line 1462 in 3ae9b9d

    override suspend fun handlePostRequest(

Copy link
Member

@david-allison david-allison Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove this from the merge queue if you're also unhappy. This was NOT a blocking comment, so feel free to re-add it. No 'hard' advice on this one, but I'd prefer it as a developer-only setting. We can expect developers to read.

I found hard doing that without losing context enough to explain the setting.

Good reason for a developer-only setting.

CORS is rejected by default

CORS is client-side security. A malicious app would skip the OPTIONS call.

Honestly, I tend to prefer just making it the default.

Hard no until we resolve

Especially with how this is currently in the tech news cycle: https://www.zeropartydata.es/p/localhost-tracking-explained-it-could

Copy link
Member Author

@BrayanDSO BrayanDSO Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an advanced setting and under the Workarounds category. I think it matches any possible expectatives from the public. Some advanced notetypes from shared decks may use it, and I don't want people going to Developer options just to enable it.

Since I'm no expert in security, and it's way harder and time consuming to defend than to attack, I'll merge it as an Advanced setting.

Copy link
Member

@mikehardy mikehardy Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good compromise - advanced ppl will really want this, but it is opt in so don't increase surface area by default


<!-- Developer options -->
<string name="pref_cat_dev_options" maxLength="41">Developer options</string>
Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/res/values/preferences.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@
<string name="pref_new_notifications">newNotifications</string>
<string name="dev_options_enabled_by_user_key">devOptionsEnabledByUser</string>
<string name="pref_cat_wip_key">workInProgressDevOptions</string>
<string name="reviewer_port_pref_key">reviewerPort</string>
<string name="use_fixed_port_pref_key">useFixedPort</string>
<!-- Developer options > Create fake notes -->
<string name="pref_fill_default_deck_number_key">FillDefaultNumberDeck</string>
<string name="pref_fill_default_deck_key">FillDefaultDeck</string>
Expand Down
5 changes: 5 additions & 0 deletions AnkiDroid/src/main/res/xml/preferences_advanced.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@
android:key="@string/media_import_allow_all_files_key"
android:summary="@string/media_import_allow_all_files_summ"
android:title="@string/media_import_allow_all_files" />
<SwitchPreferenceCompat
android:title="@string/pref_fixed_port_title"
android:key="@string/use_fixed_port_pref_key"
android:summary="@string/pref_fixed_port_summary"
android:defaultValue="false"/>
</PreferenceCategory>
<PreferenceCategory
android:key="@string/pref_cat_plugins_key"
Expand Down
Loading