Navigation component not accepting arguments in Dialogs that doesn't inherit from DialogFragment directly [168584987]

Fixed

Bug

Status Update

No update yet.

Description

ca...@gmail.com

created issue #1

Sep 15, 2020 05:57PM

Component used:
- androidx.navigation:navigation-fragment-ktx:2.2.1
- androidx.navigation:navigation-ui-ktx:2.2.1
Version used:
Devices/Android versions reproduced on: Cannot compile

STEPS:

Create a DialogFragment class called MyDialog
Now create another class called MyNewDialog that inherits from MyDialog instead of DialogFragment(for example, to inherit styles or UI)

Now in a XML graph, try to add it and put it some arguments:

<dialog
    android:id="@+id/my_new_dialog"
    android:name="com.example.app.dialogs.MyNewDialog"
    tools:layout="@layout/my_dialog">

    <argument
        android:name="argument_1"
        app:argType="string"/>

    <action
        android:id="@+id/action_1"
        app:destination="@id/destination_1" />

</dialog>

Then, when trying to build the application, it won't compile and the navigation generated code will throw:

This function must return a value of type MyNewDialogArgs

However, no problem with actions (Why it doesn't throw same error with type MyNewDialogDirections??)

No problem if we use MyDialog(that inherits directly from DialogFragment) instead of MyNewDialog(that inherits from MyDialogand this last, from DialogFragment

Comments

il...@google.com <il...@google.com> #2Sep 15, 2020 06:28PM

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

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

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

ca...@gmail.com <ca...@gmail.com> #3Sep 16, 2020 01:06PM

Hello again!

It was very tough but trying to replicate the issue in a sample project (since original project I cannot share it) I finally found the source of the issue and it's related to the length of the names of the objects used when generating code by Navigation component and line wrapping in those files

Please, try to compile project that I've attached and you will see the error. So no problem with inheritance or other possible causes that I commented in first message.

In project example, I've created:

MyNewDialogWithAVeryLongNameToTriggerAnError that inhertis from DialogFragment

Then I've added it to in nav_graph_test.xmland with arguments that have also long IDs too:

<dialog
    android:id="@+id/very_large_name_for_my_new_dialog_widget_for_trigger_an_error"
    android:name="com.example.navtestapp.MyNewDialogWithAVeryLongNameToTriggerAnError"
    tools:layout="@layout/fragment_my_dialog">

    <argument
        android:name="very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error"
        app:argType="string"
        app:nullable="true"/>

    <argument
        android:name="second_very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error"
        app:argType="string"
        app:nullable="true"/>

    <action
        android:id="@+id/action_dialog"
        app:destination="@id/my_fragment" />

</dialog>

Then, when we try to compile project, we can't. It returns the error comented in first message:

This function must return a value of type MyNewDialogWithAVeryLongNameToTriggerAnErrorArgs

If we look at generated code we can see:

data class MyNewDialogWithAVeryLongNameToTriggerAnErrorArgs(
  val veryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError: String?,
  val secondVeryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError: String?
) : NavArgs {
  fun toBundle(): Bundle {
    val result = Bundle()
    result.putString("very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error",
        this.veryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError)
    result.putString("second_very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error",
        this.secondVeryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError)
    return result
  }

  companion object {
    @JvmStatic
    fun fromBundle(bundle: Bundle): MyNewDialogWithAVeryLongNameToTriggerAnErrorArgs {
      bundle.setClassLoader(MyNewDialogWithAVeryLongNameToTriggerAnErrorArgs::class.java.classLoader)
      val __veryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError : String?
      if
          (bundle.containsKey("very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error")) {
        __veryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError =
            bundle.getString("very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error")
      } else {
        throw IllegalArgumentException("Required argument \"very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error\" is missing and does not have an android:defaultValue")
      }
      val __secondVeryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError : String?
      if
          (bundle.containsKey("second_very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error")) {
        __secondVeryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError =
            bundle.getString("second_very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error")
      } else {
        throw IllegalArgumentException("Required argument \"second_very_large_name_for argument_in_my_new_dialog_widget_for_trigger_an_error\" is missing and does not have an android:defaultValue")
      }
      return
          MyNewDialogWithAVeryLongNameToTriggerAnErrorArgs(__veryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError,
          __secondVeryLargeNameForArgumentInMyNewDialogWidgetForTriggerAnError)
    }
  }
}

Pay attention to return in fromBundle() function that is split into two lines, preventing project to compile since compiler expects an expresion for returning it but it's in the next line. Removing breakline between return and MyNewDialogWithAVeryLongNameToTriggerAnErrorArgs fixes the problem, but since this is generated code, we are not able to do it.

Shouldn't navigation component take into consideration that objects could have long names and ensure that code generated finally is able to compile it?

Thank you!

NavTestApp.zip

26 MB

Download

il...@google.com <il...@google.com> #4Sep 16, 2020 04:21PM

Assigned to jb...@google.com.

Thanks, your reproduction project and additional analysis is super helpful.

Safe Args uses a project called KotlinPoet that actually generates the Kotlin code and it is that project that controls line breaks.

We'll look at if this is still reproducible with the latest version of KotlinPoet. If upgrading fixes it, we'll include that upgrade in Navigation 2.3.1.

Otherwise, we'll file an issue against KotlinPoet and have to wait for a release that includes a fix.

ap...@google.com <ap...@google.com> #5Sep 24, 2020 01:37AM

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

commit 766a3ad47da73e1179ce6419693a825c4d665cfd
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

jb...@google.com <jb...@google.com> #6Sep 24, 2020 08:37AM

Marked as fixed.

This has been fixed internally and will be available in the Navigation 2.3.1 release.

ap...@google.com <ap...@google.com> #7Sep 24, 2020 09:34AM

Project: platform/frameworks/support
Branch: snap-temp-L34200000699848945

commit 7313c5c84065d19bde995abd6abdbc49054ce96e
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625
(cherry picked from commit 766a3ad47da73e1179ce6419693a825c4d665cfd)

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

ap...@google.com <ap...@google.com> #8Sep 24, 2020 09:36AM

Project: platform/frameworks/support
Branch: snap-temp-L04700000699849955

commit a7aa9b8a2bd4a2f038b700d1ad57feea325ea137
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625
(cherry picked from commit 766a3ad47da73e1179ce6419693a825c4d665cfd)

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

ap...@google.com <ap...@google.com> #9Sep 24, 2020 10:08AM

Project: platform/frameworks/support
Branch: snap-temp-L69500000699869549

commit 658ded1d1c845cc764fb47e07c8f560ffae406ff
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625
(cherry picked from commit 766a3ad47da73e1179ce6419693a825c4d665cfd)

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

ap...@google.com <ap...@google.com> #10Sep 24, 2020 10:10AM

Project: platform/frameworks/support
Branch: snap-temp-L31300000699869852

commit 998afa5a45306f1096a053e0f9eb275481fe79ab
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625
(cherry picked from commit 766a3ad47da73e1179ce6419693a825c4d665cfd)

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

ap...@google.com <ap...@google.com> #11Sep 24, 2020 11:30AM

Project: platform/frameworks/support
Branch: snap-temp-L53700000699921463

commit 0e594474451b1e1a2f6927179c703f1547dbfe7c
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625
(cherry picked from commit 766a3ad47da73e1179ce6419693a825c4d665cfd)

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

ap...@google.com <ap...@google.com> #12Sep 24, 2020 11:32AM

Project: platform/frameworks/support
Branch: snap-temp-L25200000699921867

commit c9e5aa95317da82d339c362a3dae64a9f331e214
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625
(cherry picked from commit 766a3ad47da73e1179ce6419693a825c4d665cfd)

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

ap...@google.com <ap...@google.com> #13Sep 24, 2020 11:52AM

Project: platform/frameworks/support
Branch: snap-temp-L07700000699933785

commit f3069dc913eb8d41e6cc3859df5d6ef204fdf18c
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 23 14:44:52 2020

Keep long arg names from wrapping

When using safe-args with a long argument name, kotlin does an
automatic line wrap at 100 characters which makes it place the argument
return and variable on different lines, which fails.

We should add `·` to emit a space that never wraps.

Test: Added KotlinNavWriterTest
Bug: 168584987
Change-Id: Ibc31f91fd6b1c4a94f0b7c05b9b9679ffa09b625
(cherry picked from commit 766a3ad47da73e1179ce6419693a825c4d665cfd)

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/ReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongNameMainFragmentArgs.kt

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

ma...@syslogic.io <ma...@syslogic.io> #14Jun 3, 2022 01:44PM

Just gotten into a similar situation (the title matches): When calling method getArguments() in the constructor, it returns null. While in public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) it returns the Bundle Alike this I can obtain the arguments of a DialogFragment inside a library, which doesn't know about the Navigation at all.