From 561d7d630718acb8dca57e15f5af55b9abcfc83e Mon Sep 17 00:00:00 2001 From: David Moravek Date: Wed, 30 Sep 2026 15:03:10 +0200 Subject: [PATCH 1/2] fix(android): commit the portal fragment only once its container view is attached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PortalViewManager.createFragment committed the PortalFragment transaction asynchronously. FragmentManager resolves the container view by id when the transaction executes on the next main-loop iteration and throws IllegalArgumentException: No view found for id 0x… for fragment PortalFragment when the container is not part of the activity's window at that moment: - React dropped the PortalView before the transaction ran (mount and unmount in the same batch). onDropViewInstance could not cancel the pending add, because Fragment.parentFragmentManager throws until the add has executed and the catch swallowed it. - The view exists but is not attached yet, e.g. a react-native-screens screen that is still animating in when the PortalView mounts (seen in production when a portal screen is re-opened with a cached portal config), or it was detached while React still has the subtree mounted. Commit the transaction with commitNowAllowingStateLoss from a Runnable posted on the container view instead. View.post defers the runnable until the view is attached and always dispatches it through the main handler, so the commit never nests inside another FragmentManager transaction. When it runs, it re-checks that the view was not dropped and is attached. viewState.fragment is only set after a successful add, so onDropViewInstance can use parentFragmentManager without the try/catch. Co-Authored-By: Claude Fable 5.1 --- .../ionic/portals/reactnative/PortalView.kt | 64 +++++++++++++------ 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/android/src/main/java/io/ionic/portals/reactnative/PortalView.kt b/android/src/main/java/io/ionic/portals/reactnative/PortalView.kt index 10ce9e2..4e1ae39 100644 --- a/android/src/main/java/io/ionic/portals/reactnative/PortalView.kt +++ b/android/src/main/java/io/ionic/portals/reactnative/PortalView.kt @@ -113,7 +113,6 @@ internal class PortalViewManager(private val context: ReactApplicationContext) : val portalFragment = PortalFragment(portal) viewState.initialContext?.let(portalFragment::setInitialContext) - viewState.fragment = portalFragment portalFragment.lifecycle.addObserver(object : LifecycleEventObserver { override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { @@ -130,28 +129,55 @@ internal class PortalViewManager(private val context: ReactApplicationContext) : } }) - val activity = context.currentActivity as? FragmentActivity ?: return - activity.supportFragmentManager - .beginTransaction() - .replace(viewId, portalFragment, "$viewId") - .commit() + // The fragment transaction must not be committed asynchronously here: + // FragmentManager resolves the container view by id when the + // transaction executes (next main-loop iteration), and throws + // "No view found for id" if the container is not part of the + // activity's window at that moment. That happens when React drops the + // view before the transaction runs (mount and unmount in one batch), + // or when the view exists but is not attached yet / any more (e.g. a + // react-native-screens screen still animating in, or detached while + // React keeps the subtree mounted). + // + // Instead, commit synchronously from a runnable posted on the view: + // - View.post() defers the runnable until the view is attached, and + // always dispatches it through the main handler, so the commit never + // nests inside another FragmentManager transaction that may be + // executing (react-native-screens commits with commitNow). + // - When it runs, re-check that the view is still ours and attached. + parentView.post(object : Runnable { + override fun run() { + if (fragmentMap[viewId] !== viewState) return // dropped meanwhile + if (!parentView.isAttachedToWindow) { + parentView.post(this) // re-queued until the next attach + return + } + val activity = context.currentActivity as? FragmentActivity ?: return + try { + activity.supportFragmentManager + .beginTransaction() + .replace(viewId, portalFragment, "$viewId") + .commitNowAllowingStateLoss() + viewState.fragment = portalFragment + } catch (e: IllegalStateException) { + // Host destroyed or FragmentManager unavailable. + Log.i("io.ionic.portals.rn", "Fragment manager not available", e) + } + } + }) } override fun onDropViewInstance(view: FrameLayout) { super.onDropViewInstance(view) - val viewState = fragmentMap[view.id] ?: return - - try { - viewState.fragment - ?.parentFragmentManager - ?.beginTransaction() - ?.remove(viewState.fragment!!) - ?.commit() - } catch (e: IllegalStateException) { - Log.i("io.ionic.portals.rn", "Parent fragment manager not available") - } - - fragmentMap.remove(view.id) + val viewState = fragmentMap.remove(view.id) ?: return + // Only set once the fragment was actually added, so parentFragmentManager + // is available here. A drop before the add ran is handled by the + // identity check in the posted runnable above. + val fragment = viewState.fragment ?: return + fragment.parentFragmentManager + .beginTransaction() + .remove(fragment) + .commitAllowingStateLoss() } private fun setupLayout(view: ViewGroup) { From 735a488d54adbf04a0180ea47e2230ce5f7c50a2 Mon Sep 17 00:00:00 2001 From: David Moravek Date: Wed, 30 Sep 2026 15:28:54 +0200 Subject: [PATCH 2/2] test(android): add Robolectric regression tests for fragment attachment Extract the "commit once the container is attached and still ours" logic into runWhenAttached so it can be exercised without PortalFragment (which would boot a Capacitor bridge and WebView), and cover it with Robolectric against the real androidx FragmentManager: - control test reproducing the production failure: an async commit whose container was removed before it ran throws "No view found for id" - commits once the looper runs when the container is attached - skips the commit when the view was dropped before it ran - waits for the container to attach before committing - does not throw when the container is detached while still current, and commits once it comes back The library module had no unit test setup; this adds junit + robolectric as test dependencies and a dedicated test-android CI job that runs testDebugUnitTest on every push. It is a separate job on purpose: the build-android job is replayed from the turbo cache whenever yarn.lock is unchanged, so a step gated on that cache would be skipped exactly when android/ changed. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/ci.yml | 38 +++++ android/build.gradle | 9 ++ .../ionic/portals/reactnative/PortalView.kt | 41 ++---- .../portals/reactnative/RunWhenAttached.kt | 27 ++++ .../reactnative/RunWhenAttachedTest.kt | 136 ++++++++++++++++++ 5 files changed, 225 insertions(+), 26 deletions(-) create mode 100644 android/src/main/java/io/ionic/portals/reactnative/RunWhenAttached.kt create mode 100644 android/src/test/java/io/ionic/portals/reactnative/RunWhenAttachedTest.kt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 558b039..a59a7d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,6 +103,44 @@ jobs: run: | yarn turbo run build:android --cache-dir="${{ env.TURBO_CACHE_DIR }}" + test-android: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v5 + + - name: Setup + uses: ./.github/actions/setup + + - name: Install JDK + uses: actions/setup-java@v4 + with: + distribution: 'zulu' + java-version: '17' + + - name: Finalize Android SDK + run: | + /bin/bash -c "yes | $ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager --licenses > /dev/null" + + - name: Cache Gradle + uses: actions/cache@v4 + with: + path: | + ~/.gradle/wrapper + ~/.gradle/caches + key: ${{ runner.os }}-gradle-${{ hashFiles('example/android/gradle/wrapper/gradle-wrapper.properties') }} + restore-keys: | + ${{ runner.os }}-gradle- + + - name: Run Android unit tests + run: | + cd example/android + ./gradlew :ionic_portals-react-native:testDebugUnitTest --no-daemon --console=plain + + - name: Summarize unit test results + if: always() + run: grep -h " Boolean, action: () -> Unit) { + view.post(object : Runnable { + override fun run() { + if (!isCurrent()) return + if (!view.isAttachedToWindow) { + view.post(this) + return + } + action() + } + }) +} diff --git a/android/src/test/java/io/ionic/portals/reactnative/RunWhenAttachedTest.kt b/android/src/test/java/io/ionic/portals/reactnative/RunWhenAttachedTest.kt new file mode 100644 index 0000000..353b90a --- /dev/null +++ b/android/src/test/java/io/ionic/portals/reactnative/RunWhenAttachedTest.kt @@ -0,0 +1,136 @@ +package io.ionic.portals.reactnative + +import android.os.Looper +import android.view.View +import android.view.ViewGroup +import android.widget.FrameLayout +import androidx.fragment.app.Fragment +import androidx.fragment.app.FragmentActivity +import androidx.fragment.app.FragmentManager +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +/** + * Regression tests for the container race behind + * `IllegalArgumentException: No view found for id … for fragment PortalFragment`. + * + * They drive the real androidx [FragmentManager] under Robolectric with plain + * [Fragment]s, which is enough: `FragmentStateManager.createView` resolves the + * container by id before it asks the fragment for a view. `PortalFragment` + * itself is out of scope here, it would boot a Capacitor bridge and a WebView. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class RunWhenAttachedTest { + private lateinit var activity: FragmentActivity + private lateinit var container: FrameLayout + + private val fragmentManager: FragmentManager + get() = activity.supportFragmentManager + + @Before + fun setUp() { + activity = Robolectric.buildActivity(FragmentActivity::class.java).setup().get() + container = FrameLayout(activity).apply { id = View.generateViewId() } + } + + private fun attachContainer() = activity.setContentView(container) + + private fun detachContainer() = (container.parent as ViewGroup).removeView(container) + + private fun idleMainLooper() = shadowOf(Looper.getMainLooper()).idle() + + private fun commitFragmentNow() { + fragmentManager + .beginTransaction() + .replace(container.id, Fragment()) + .commitNowAllowingStateLoss() + } + + private fun fragmentInContainer(): Fragment? = fragmentManager.findFragmentById(container.id) + + /** + * Control: this is the pattern the view manager used before the fix and the + * production crash. It proves the harness reproduces the failure, so the + * tests below mean something. + */ + @Test + fun asyncCommitThrowsWhenTheContainerWasRemovedBeforeItRan() { + attachContainer() + fragmentManager.beginTransaction().replace(container.id, Fragment()).commit() + detachContainer() + + val thrown = runCatching { idleMainLooper() }.exceptionOrNull() + + assertNotNull("expected the pending add to throw", thrown) + val messages = generateSequence(thrown) { it.cause } + .mapNotNull { it.message } + .joinToString(" | ") + assertTrue(messages, messages.contains("No view found for id")) + } + + @Test + fun commitsOnceTheLooperRunsWhenTheContainerIsAttached() { + attachContainer() + + runWhenAttached(container, isCurrent = { true }, action = ::commitFragmentNow) + idleMainLooper() + + assertTrue(fragmentInContainer()?.isAdded == true) + } + + @Test + fun skipsTheCommitWhenTheViewWasDroppedBeforeItRan() { + attachContainer() + var actionRuns = 0 + + runWhenAttached(container, isCurrent = { false }) { + actionRuns++ + commitFragmentNow() + } + detachContainer() + idleMainLooper() + + assertEquals(0, actionRuns) + assertNull(fragmentInContainer()) + } + + @Test + fun waitsForTheContainerToAttachBeforeCommitting() { + // Not attached yet: mirrors a screen that is still animating in. + runWhenAttached(container, isCurrent = { true }, action = ::commitFragmentNow) + idleMainLooper() + assertNull("must not commit before the container is in the window", fragmentInContainer()) + + attachContainer() + idleMainLooper() + + assertTrue(fragmentInContainer()?.isAdded == true) + } + + @Test + fun doesNotThrowWhenTheContainerIsDetachedWhileStillCurrent() { + // Mirrors a screen detached natively while React still has the view mounted. + attachContainer() + runWhenAttached(container, isCurrent = { true }, action = ::commitFragmentNow) + detachContainer() + + idleMainLooper() + + assertNull(fragmentInContainer()) + + // ...and the commit still happens once the container comes back. + attachContainer() + idleMainLooper() + assertTrue(fragmentInContainer()?.isAdded == true) + } +}