streetcomplete / StreetComplete

Easy to use OpenStreetMap editor for Android
https://streetcomplete.app
GNU General Public License v3.0
3.82k stars 348 forks source link

Crash when a dialog in a form for newly created element is shown while uploading #5809

Open Helium314 opened 3 weeks ago

Helium314 commented 3 weeks ago

How to Reproduce

  1. enable auto-upload
  2. make sure you're offline
  3. split a road
  4. enable sidewalk overlay and edit the new way (with negative id, I think usually it's the shorter one, or the one with fewer nodes)
  5. tap a side to bring up the sidewalk selection dialog
  6. go online and wait until answers are uploaded (you may notice the quest form disappearing, while the dialog stays open)
  7. select a sidewalk answer in the dialog
  8. enjoy a crash
stack trace ``` FATAL EXCEPTION: main Process: de.westnordost.streetcomplete.debug, PID: 28490 java.lang.NullPointerException at de.westnordost.streetcomplete.overlays.AbstractOverlayForm.getBinding(AbstractOverlayForm.kt:117) at de.westnordost.streetcomplete.overlays.AbstractOverlayForm.checkIsFormComplete(AbstractOverlayForm.kt:331) at de.westnordost.streetcomplete.overlays.AStreetSideSelectOverlayForm.onViewCreated$lambda$0(AStreetSideSelectOverlayForm.kt:41) at de.westnordost.streetcomplete.overlays.AStreetSideSelectOverlayForm.$r8$lambda$4U5UEE9L5KQfbzWrn_qlOa3utbY(Unknown Source:0) at de.westnordost.streetcomplete.overlays.AStreetSideSelectOverlayForm$$ExternalSyntheticLambda0.invoke(D8$$SyntheticClass:0) at de.westnordost.streetcomplete.view.controller.StreetSideSelectWithLastAnswerButtonViewController.replacePuzzleSide(StreetSideSelectWithLastAnswerButtonViewController.kt:173) at de.westnordost.streetcomplete.overlays.sidewalk.SidewalkOverlayForm.onClickSide$lambda$1(SidewalkOverlayForm.kt:41) at de.westnordost.streetcomplete.overlays.sidewalk.SidewalkOverlayForm.$r8$lambda$XmYPk6VHenUBzl1LhA1Im1kwa78(Unknown Source:0) at de.westnordost.streetcomplete.overlays.sidewalk.SidewalkOverlayForm$$ExternalSyntheticLambda0.invoke(D8$$SyntheticClass:0) at de.westnordost.streetcomplete.view.image_select.ImageListPickerDialog$1.onIndexSelected(ImageListPickerDialog.kt:40) at de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter.select$lambda$2(ImageSelectAdapter.kt:62) at de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter.$r8$lambda$_F7RuzH7SSEqeJhLTLyJKreHbaE(Unknown Source:0) at de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter$$ExternalSyntheticLambda0.invoke(D8$$SyntheticClass:0) at de.westnordost.streetcomplete.util.Listeners.forEach(Listeners.kt:24) at de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter.select(ImageSelectAdapter.kt:62) at de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter.toggle(ImageSelectAdapter.kt:76) at de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter$onCreateViewHolder$1.invoke(ImageSelectAdapter.kt:44) at de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter$onCreateViewHolder$1.invoke(ImageSelectAdapter.kt:44) at de.westnordost.streetcomplete.view.image_select.ItemViewHolder._set_onClickListener_$lambda$0(ItemViewHolder.kt:37) at de.westnordost.streetcomplete.view.image_select.ItemViewHolder.$r8$lambda$NhmaengEeC7TKKkep-8v9Ewpgi0(Unknown Source:0) at de.westnordost.streetcomplete.view.image_select.ItemViewHolder$$ExternalSyntheticLambda0.onClick(D8$$SyntheticClass:0) at android.view.View.performClick(View.java:7259) at android.view.View.performClickInternal(View.java:7236) at android.view.View.access$3600(View.java:801) at android.view.View$PerformClick.run(View.java:27892) at android.os.Handler.handleCallback(Handler.java:883) at android.os.Handler.dispatchMessage(Handler.java:100) at android.os.Looper.loop(Looper.java:214) at android.app.ActivityThread.main(ActivityThread.java:7356) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:491) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:940) ```

Expected Behavior Dialog should close automatically, or at least there should be no crash on selecting something.

Versions affected current master (a6c49bc4a845d002e174d3ab1910cbc07e6850e9)

westnordost commented 3 weeks ago

Hmm, the case of a dialog staying open when the fragment closes would be really a lot of work to solve. Basically, any quest form would need to keep track of its dialogs opened and then close them all on onDestroy.

(In Compose, this is no problem)

So, maybe something could be done here regarding why this form is closed in the first place.

But eh... the open quest form has the element ID of the currently selected element. After upload, the ID changed (because it was a created element) and thus MainFragment::onReplacedForBbox detects this as "this element does not exist anymore, so close the form for it".

One solution would be to (try to) update the element id in that form on an element id update... but IMO this sounds not really worth the effort in order to circumvent such a rare crash issue.

Helium314 commented 3 weeks ago

Hmm, then compose looks like the "easiest" solution

westnordost commented 3 weeks ago

(Will be some time, though)