1907286 - IllegalStateException calling requireContext in TranslationsDialogFragment

Status: RESOLVED FIXED

(Firefox for Android :: Translations, defect, P1)

Description

[Polly [:polly]]

Spotted in crash telemetry. Stack trace is

Exception java.lang.IllegalStateException:
  at androidx.fragment.app.Fragment.requireContext (Fragment.java:972)
  at androidx.fragment.app.Fragment.getResources (Fragment.java:1036)
  at org.mozilla.fenix.translations.TranslationsDialogFragment.onCreateDialog$lambda$1$lambda$0 (TranslationsDialogFragment.java:83)
  at android.app.Dialog$ListenersHandler.handleMessage (Dialog.java:1501)
  at android.os.Handler.dispatchMessage (Handler.java:106)
  at android.os.Looper.loopOnce (Looper.java:224)
  at android.os.Looper.loop (Looper.java:318)
  at android.app.ActivityThread.main (ActivityThread.java:8772)
  at java.lang.reflect.Method.invoke
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:561)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1013)

Comment 1

[Chris Peterson [:cpeterson]]

This TranslationsDialogFragment crash was a regression in Fx 126.

Comment 2

[Olivia Hall [:olivia]]

Gabriel, do you have any insight into this bug? I'll try to figure out more too.

Comment 3

[Polly [:polly]]

Looks like we have had many previous similar issues from using requireContext().
It is likely to be difficult to reproduce but easy to fix.
I agree with Arturo's suggestion in this related crash: https://github.com/mozilla-mobile/fenix/issues/8408#issuecomment-585990538

Comment 4

[Iorga Gabriel]

I think we should use navigateSafe method when we open the TranslationsDialogFragment.
val directions =
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment()
navController.navigateSafe(R.id.browserFragment, directions)
In MenuNavigationMiddleware is used without navigateSafe like this :
is MenuAction.Navigate.Translate -> navController.nav(
R.id.menuDialogFragment,
MenuDialogFragmentDirections.actionMenuDialogFragmentToTranslationsDialogFragment(),
)
Maybe from here is the bug.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 AArch64 and ARM crashes on release

For more information, please visit BugBot documentation.

Comment 6

[Olivia Hall [:olivia]]

I'll work on a speculative fix today based on that information, thanks for the information and suggestions!

Comment 7

[Olivia Hall [:olivia]]

Attachment: Bug 1907286 - IllegalStateException calling requireContext in TranslationsDialogFragment

Attempt to fix IllegalStateException in TranslationsDialogFragment.

• Added navigateSafe to MenuNavigationMiddleware
• Added runIfFragmentIsAttached to places where findNavController().popBackStack() was used in the translations area

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab640abf079a
IllegalStateException calling requireContext in TranslationsDialogFragment r=android-reviewers,pollymce

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/ab640abf079a

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:olivia, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox129 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Olivia Hall [:olivia]]

Comment on attachment 9412503 [details]
Bug 1907286 - IllegalStateException calling requireContext in TranslationsDialogFragment

Beta/Release Uplift Approval Request

• User impact if declined: More crashes in the Android translations area.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: Not an easily tested code path to reproduce a crash.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is not particularly risky because this patch only adds guard statements to prevent trying to get a context when one is not available. However, this patch is a speculative crash fix and cannot be easily tested.
• String changes made/needed:
• Is Android affected?: Yes

Comment 12

[Donal Meehan [:dmeehan]]

Comment on attachment 9412503 [details]
Bug 1907286 - IllegalStateException calling requireContext in TranslationsDialogFragment

Approved for 129.0b5

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/eb5cf201eee7

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Release approval too.

Comment 15

[Olivia Hall [:olivia]]

Comment on attachment 9412503 [details]
Bug 1907286 - IllegalStateException calling requireContext in TranslationsDialogFragment

Beta/Release Uplift Approval Request

• User impact if declined: More crashes in the Android translations area.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: Not an easily tested code path to reproduce a crash.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is not particularly risky because this patch only adds guard statements to prevent trying to get a context when one is not available. However, this patch is a speculative crash fix and cannot be easily tested.
• String changes made/needed:
• Is Android affected?: Yes

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9412503 [details]
Bug 1907286 - IllegalStateException calling requireContext in TranslationsDialogFragment

Approved for 128.0.2

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/c870042ef7e8