java.lang.NoSuchMethodException: org.readium.r2.navigator.epub.EpubNavigatorFragment.<init> [] #395
Comments
|
Make sure you use the |
|
@qnga I'm doing almost the same, but after onCreate (because book is being loaded with viewModel and is not ready at the moment of fragment creation). In my case code looks like this: As I understand, it happens because after re-creation it tries to restore fragment, but there is no factory set yet. Unfortunately, there is no easy way to make initial data available on such early stage in my specific case. Could you please suggest any workarounds? |
|
This is tricky. I guess you could solve it by making your fragment hierarchy a bit more complex. Embed the |
|
I'm closing this issue as it works as expected in the toolkit, but feel free to continue the discussion. I would also recommend @qnga's suggestion for this. |
|
@mickael-menu Now, I updated Android Navigation from 2.5.3 to 2.6.0. That change caused the same crash to happen again. Now I made another hotfix, reverting the Android Navigation version to 2.5.3. Also, I started searching github and found this issue which is currently marked as closed. I dit a git checkout to the Reproduction steps:
Yes, this is not the same crash as reported above, but this is the first issue I see that needs to be addressed by the Readium team - related to the issue reported by @RankoR - the sample app crashes when trying to restore from process death, if an EPUB publication was opened when the system decided to kill the process. The latest toolkit release is from December 28, 2022, so it's almost a year old. I'd love to see the sample app updated, to use the latest dependencies and the crash from above fixed. I'm trying to use the sample app as a guidance, but it's impossible to do so when I update the androidx dependencies in our app to the latest versions, and suddenly crashes start appearing in the Readium toolkit internal classes. Eagerly waiting for your input, as now I have 2 android dependencies that I cannot update because of this crash. Thank you! |
|
@radusalagean It's something to fix in your application. Here's a diff that fixes the problem in the test app: diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt
index cc8867dc7..e99427446 100644
--- a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt
+++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt
@@ -49,6 +49,8 @@ open class ReaderActivity : AppCompatActivity() {
private lateinit var readerFragment: BaseReaderFragment
override fun onCreate(savedInstanceState: Bundle?) {
+ super.onCreate(savedInstanceState)
+
/*
* We provide dummy publications if the [ReaderActivity] is restored after the app process
* was killed because the [ReaderRepository] is empty.
@@ -56,10 +58,9 @@ open class ReaderActivity : AppCompatActivity() {
*/
if (model.publication.readingOrder.isEmpty()) {
finish()
+ return
}
- super.onCreate(savedInstanceState)
-
val binding = ActivityReaderBinding.inflate(layoutInflater)
setContentView(binding.root)I won't fix it in |
|
@mickael-menu That diff doesn't solve the crash (video demo). Passing the override fun onCreate(savedInstanceState: Bundle?) {
/*
* We provide dummy publications if the [ReaderActivity] is restored after the app process
* was killed because the [ReaderRepository] is empty.
* In that case, finish the activity as soon as possible and go back to the previous one.
*/
if (model.publication.readingOrder.isEmpty()) {
+ super.onCreate(null)
finish()
+ return
}
super.onCreate(savedInstanceState)
val binding = ActivityReaderBinding.inflate(layoutInflater)
setContentView(binding.root)Related to the The saved state of Fragments has been split entirely between private library state (custom Parcelable classes) and state provided by the developer, which is now always stored in a Bundle that allows determining exactly where a fragment’s state is originating. In my app, I have a single Activity and multiple fragments. So in override fun onCreate(savedInstanceState: Bundle?) {
val readerInitData = viewModel.readerInitData as? EpubReaderInitData
if (readerInitData == null) {
navController?.popBackStack()
} else {
childFragmentManager.fragmentFactory =
readerInitData.navigatorFactory.createFragmentFactory(
initialLocator = readerInitData.initialLocation,
listener = this,
initialPreferences = readerInitData.preferencesManager.preferences.value,
configuration = EpubNavigatorFragment.Configuration {
// Config stuff here
}
)
}
super.onCreate(if (readerInitData == null) null else savedInstanceState)
}Emphasis on the last line: After the library update, it tries to restore the fragment, hence the crash. I'm stuck on this, as I don't know any way to "discard" the fragment before the OS attempts to restore it. I think Android never meant for the process restoration mechanism to bypass the restoration of fragments. As far as I know, they always pushed devs to properly restore the state the user was in before the OS killed the app. For now I will keep @qnga 's suggestion does not apply here, because the system attempts to restore the fragment hierarchy, and the I understand that v3 is in development, but until that is stable and released, we need the I hope I didn't come off as rude or demanding in any way, that is not my intention, I'm just trying to explain that as far as I'm concerned, this is not something that I can fix now on my side with my current Single Activity Multi Fragment architecture.
If you agree, please reopen this issue and acknowledge it so my client has visibility of this. Thank you! |
|
Any reason you are sticking with I agree this should be addressed in the toolkit though, I'm reopening this issue. However, I'm still unsure about the proper way to fix it using an API change. What would the ideal API look like to you?
Let's say you provide a suspending closure to return a |
mickael-menu
added
bug
That workaround I showed in my previous comment works in the context of the Readium Sample app, which uses a dedicated Reader Activity. Calling
I understand your point - and I agree, preventing restoration of fragments in the hierarchy is preferred in this case, as the alternative would probably be more prone to bugs and involve signigicant changes in
If you can find a way so we can start updating If you're asking me about the ideal API, that is something that I cannot answer without spending more hours studying your codebase and implications of my suggestions. That would be an architectural assistance request, which means effectively working on the Readium project, something that I'm not currently in the position to do. However, the ideal API is something that you and your team can establish by following Android's recommendations when building it, such as not expecting data that cannot be passed synchronously (i.e. the publication) in the constructor of the fragment. Another idea that requires more work is to modify the I prepared the repo with the bare minimum changes so you can reproduce the crash. You can find the commit here: radusalagean@8131d65 |
Do you know a way to prevent fragment restoration since We're aware of the shortcomings of the current Navigators, which extend beyond this particular issue. That's why we've been prototyping a new version from the ground up, which is a substantial task. Meanwhile, we need a solution that does not require a major overhaul of the current navigators. Would something like that work for you? It initializes the navigator with a dummy publication, but it's still the responsibility of the app to ditch the navigator fragment after restoring the app before the view is displayed on the screen. It seems to address the issue with the commit you shared above. |
|
@mickael-menu I tested your change in my project and can confirm that it works. The override fun onCreate(savedInstanceState: Bundle?) {
val readerInitData = viewModel.readerInitData as? EpubReaderInitData
if (readerInitData == null) {
navController?.popBackStack() // This
} else {
...
}
...
}Thus, when restoring the app, the lifecycle logs show: So we are fine. This patch also needs to be applied to the |
|
Sure, thank you for testing the fix and bringing this issue to our attention. |
|
@radusalagean Would you mind confirming these changes work for you with PDF too? #418 I changed the strategy because we can't create a PDF fragment without knowing the PDF engine. Now you need to create a dummy fragment factory, e.g. override fun onCreate(savedInstanceState: Bundle?) {
val publication = model.publication ?: run {
childFragmentManager.fragmentFactory = EpubNavigatorFragment.createDummyFactory()
super.onCreate(savedInstanceState)
requireActivity().finish()
// or
navController?.popBackStack()
return
}
// Create the real navigator factory as usual...
} |
|
@mickael-menu I imported all the changes from the linked pull request in my project. EPUBs still work fine with the new dummy fragment factory approach. When restoring the app if it had a PDF open prior to the OS killing the app, I'm getting |
|
Did you use the dummy PDF fragment factory too? Could you share your |
|
Yes, I did. I think the problem is that the nested @OptIn(ExperimentalReadiumApi::class)
class PDFReaderFragment : BaseReaderFragment(), PdfNavigatorFragment.Listener {
override lateinit var navigator: PdfNavigatorFragment<PdfiumSettings, PdfiumPreferences>
override fun onCreate(savedInstanceState: Bundle?) {
val readerInitData = viewModel.readerInitData as? PdfReaderInitData
if (readerInitData == null) {
childFragmentManager.fragmentFactory = PdfNavigatorFragment.createDummyFactory(
pdfEngineProvider = PdfiumEngineProvider()
)
navController?.popBackStack()
} else {
childFragmentManager.fragmentFactory = readerInitData.navigatorFactory
.createFragmentFactory(
initialLocator = readerInitData.initialLocation,
initialPreferences = readerInitData.preferencesManager.preferences.value,
listener = this
)
}
super.onCreate(if (readerInitData == null) null else savedInstanceState)
}
override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View {
val view = super.onCreateView(inflater, container, savedInstanceState)
if (savedInstanceState == null) {
childFragmentManager.commitNow {
add(
R.id.fragment_reader_container,
PdfNavigatorFragment::class.java,
Bundle(),
NAVIGATOR_FRAGMENT_TAG
)
}
}
@Suppress("Unchecked_cast")
navigator = childFragmentManager
.findFragmentByTag(NAVIGATOR_FRAGMENT_TAG) as PdfNavigatorFragment<PdfiumSettings, PdfiumPreferences>
return view
}
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
@Suppress("Unchecked_cast")
(viewModel.settings as UserPreferencesViewModel<PdfiumSettings, PdfiumPreferences>)
.bind(navigator, viewLifecycleOwner)
}
override fun onResourceLoadFailed(link: Link, error: Resource.Exception) {
Bugsnag.notify(error)
val message = when (error) {
is Resource.Exception.OutOfMemory -> "The PDF is too large to be rendered on this device"
else -> "Failed to render this PDF"
}
Toast.makeText(requireActivity(), message, Toast.LENGTH_LONG).show()
navController?.popBackStack()
}
companion object {
const val NAVIGATOR_FRAGMENT_TAG = "pdf_navigator"
}
} |
|
Creating a default constructor in |
|
Ha yes, I didn't try from your fork. It should work now. |
|
All good from my side now 🙏🏻 |
|
2.4.0 is released. Take a look at the migration guide as Readium is now distributed through Maven Central. |
Bug Report
What happened?
Android application crashes, probably on configuration changes (the easiest way to reproduce is to rotate the screen).
Orientation changes are disabled in my app, but as I see from Crashlytics it still crashes, probably on some other config changes.
Expected behavior
App doesn't crash
How to reproduce?
Rotate the screen
Environment
Development environment
Testing device
Any device actually
Additional context
The text was updated successfully, but these errors were encountered: