readium / kotlin-toolkit

A toolkit for ebooks, audiobooks and comics written in Kotlin
https://readium.org/kotlin-toolkit
BSD 3-Clause "New" or "Revised" License
193 stars 84 forks source link

Resuming the app is closing the reader screen #440

Closed LubomirGanchev closed 9 months ago

LubomirGanchev commented 10 months ago

Describe the bug

We are upgrading our project to version 2.4.0 and came across a strange behaviour. If the app is minimized when the reader is opened and then open the app again from the app drawer, the app is resumed from the previous screen (bookshelf fragment for example) it's like closing the reader screen. If the app is resumed via recent task menu, it's working as expected. Reproduced on the test app - version 2.4.0 (it's fine on 2.3.0).

How to reproduce?

  1. Open book and minimize the app
  2. Resume the app by clicking it in the app drawer (it is working as expected if resumed from recent task menu)

Readium version

2.4.0

Android API version

33

Additional context

No response

stevenzeck commented 10 months ago

It's been like this for a while actually, from this issue in the old test app repository: https://github.com/readium/kotlin-toolkit/issues/245. You can try changing the android:clearTaskOnLaunch attribute in the app's AndroidManifest.xml file. to false, but I don't know if that would leave to further issues.

mickael-menu commented 10 months ago

We don't experience this problem on Aldiko, despite using android:clearTaskOnLaunch="true". We're using an alpha version of 3.0, but I believe it's irrelevant. This is likely a regression in the test app, not the toolkit. Currently unsure of the cause, a git bisect may be necessary.

EDIT: We don't have this flag in Aldiko, I was looking at the wrong source.

LubomirGanchev commented 10 months ago

Hi @mickael-menu. We do experience it in our app too (2.4.0) I just gave the test app as an example. I saw the issue linked by @stevenzeck, but since upgrading to the latest version is the first time we experience it, never before that.

mickael-menu commented 10 months ago

I identified the commit that introduced the regression between 2.3.0 and 2.4.0. It's part of the Test App code in https://github.com/readium/kotlin-toolkit/pull/333/commits/92cafbf66c3c63701510ef6964df182aad8dbc7d, but I'm not sure exactly which part. Any idea @qnga?

@LubomirGanchev Which portions of the Test App did you copy when updating to 2.4.0?

LubomirGanchev commented 10 months ago

I copied a lot of things, because I actually upgraded from 2.2.1 (skipped 2.3.0). Unfortunately the changes between these two versions were significant and can't really tell you one by one exactly what I copied.

qnga commented 9 months ago

I agree with Steven, clearTaskOnLaunch seems to be the solution. Setting it to true looks to be specifically designed to get the behavior we don't want. No idea why it used to work before. We should switch question to "why would we need this?". In the testapp, I see no good reason.

qnga commented 9 months ago

Thinking again, do we really not want this behaviour? It doesn't look absurd to me. If you open the app from the home, that's because you forget that you used it recently and want to start again from scratch, so going back to the library makes some sense. This behavior doesn't seem to be very common in my apps though, even when the same reasoning could apply (it's not the default one after all).

mickael-menu commented 9 months ago

@LubomirGanchev You need to remove the android:clearTaskOnLaunch flag from your AndroidManifest.xml, see the rationale in the PR https://github.com/readium/kotlin-toolkit/pull/459.

LubomirGanchev commented 9 months ago

Hey @mickael-menu. I looked into this and saw that we don't have android:clearTaskOnLaunch in our manifest

mickael-menu commented 9 months ago

This fixed the issue in the Test App. I'm not sure what the problem is in your application.