NavHostFragment crashes when a custom DialogNavigator is used [175979140]

Fixed

Bug

Status Update

No update yet.

Description

ru...@gmail.com

created issue #1

Dec 20, 2020 09:20PM

Component used: Navigation
Version used: 2.3.2

Caused by:
- NavHostFragment looks up a DialogFragmentNavigator in "onAttachFragment" and no null check is done
- A custom Dialog fragment navigator can't extend from DialogFragmentNavigator as it's a final class

Commit that introduced this:

https://android.googlesource.com/platform/frameworks/support/+/2674709cf3f9d2d46c9bcee55ec01eb115143aaa

Can the new API be used instead to fix this issue in the next alpha/beta release?
In alternative, do you suggest other workaround or should we wait and stay in 2.3.1 until this is fixed?

Comments

il...@google.com <il...@google.com> #2Dec 23, 2020 11:57PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019

Change the way to detect mismatch between POJO and query

This fixes cursor mismatch warnings with expandProjection.

Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8

M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt

https://android-review.googlesource.com/1123258

https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1

ru...@gmail.com <ru...@gmail.com> #3Dec 28, 2020 06:55PM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6093116828680192/#/cherrypickHistory/all)
Created: 2019-10-14 17:23:42.610596+00:00
Changes: aosp/1123258

ru...@gmail.com <ru...@gmail.com> #4Jan 5, 2021 05:09PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020

Re-factor how errors are dismissed when query is re-written

This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (

b/140759491

).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.

This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.

Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f

M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt

https://android-review.googlesource.com/1288456

ru...@gmail.com <ru...@gmail.com> #5Feb 8, 2021 11:04PM

Any update on this? This is preventing us from updating to newer releases

il...@google.com <il...@google.com> Apr 6, 2021 07:45PM

Assigned to il...@google.com.

ap...@google.com <ap...@google.com> #6Apr 7, 2021 04:10PM

Project: platform/frameworks/support
Branch: androidx-main

commit ffe68e0a3be1da83217fadd4e40c5abad4f72b82
Author: Ian Lake <ilake@google.com>
Date: Tue Apr 06 12:40:35 2021

Use FragmentOnAttachListener in DialogFragmentNavigator

Rather than having the NavHostFragment
explicitly look for the DialogFragmentNavigator
to forward the fragment attach callback,
have DialogFragmentNavigator use the
Fragment 1.3 FragmentOnAttachListener API
to fully contain that logic within the
DialogFragmentNavigator.

Relnote: "`NavHostFragment` now supports custom
Navigators that use the same `@Navigator.Name(\"dialog\")`
as the default `DialogFragmentNavigator`."
Test: existing tests still pass
BUG: 175979140

Change-Id: Ib1c2cc38e7a621d133be1f4c3692e62913450dda

M navigation/navigation-fragment/api/public_plus_experimental_current.txt
M navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/DialogFragmentNavigator.kt
M navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/NavHostFragment.kt

https://android-review.googlesource.com/1665820

il...@google.com <il...@google.com> #7Apr 7, 2021 05:31PM

Marked as fixed.

In the upcoming Navigation 2.4.0-alpha01, we've updated our Fragment dependency such that we can use the Fragment 1.3.0's API of FragmentOnAttachListener to handle the case where previously the NavHostFragment needed to be involved. Thus, NavHostFragment no longer needs to cast the Navigator returned by the dialog name to DialogNavigator.

As a workaround, your custom Navigator can use any name other than dialog (say, singleDialog) and it'll work just fine if you use that instead of dialog.

ru...@gmail.com <ru...@gmail.com> #8Apr 7, 2021 05:49PM

Thank you for fixing this. Would singleDialog have to be also used in the navigation resources files? I didn't know we could create custom navigation targets with different names outside of dialog, fragment and activity

il...@google.com <il...@google.com> #9Apr 7, 2021 05:54PM

The default DialogFragmentNavigator, FragmentNavigator, and ActivityNavigator use the exact same @Navigator.Name API as is available to every custom Navigator (there's no special casing on the Navigation library side), so yes, it can also be used from XML.