1891972 - Allow CFRs to be dismissed by outside touches - consitent with review checker CFR behaviour

Status: RESOLVED FIXED

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

Description

[twhite]

This is an interim improvement to the current CFR behaviour

Comment 1

[twhite]

Attachment: Bug 1891972 - Allow CFRs to be dismissed by outside touches. Interim fix to improve the current CFR implementation

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5519716422b3
Allow CFRs to be dismissed by outside touches. Interim fix to improve the current CFR implementation r=android-reviewers,vdreghici

Comment 3

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5519716422b3

Comment 4

[Serban Stanca [:SerbanS]]

Backed out changeset 5519716422b3 (Bug 1891972) as requested for causing Bug 1893003.

• Backout link

Comment 5

[Serban Stanca [:SerbanS]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/5c3d6213cc73e9613d72a932412149380fa6909b

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df5c0de8443f
Allow CFRs to be dismissed by outside touches. Interim fix to improve the current CFR implementation r=android-reviewers,vdreghici

Comment 7

[tszentpeteri]

Backed out for causing requested by twhite for causing robotest failures

• Backout link
• Push with failures
• Failure Log link
• Failure log:

[task 2024-05-21T08:18:19.695Z] Command /usr/bin/java -jar /builds/worker/test-tools/flank.jar android run --config ./automation/taskcluster/androidTest/flank-arm-robo-test.yml --app /builds/worker/checkouts/gecko/mobile/android/fenix/app.apk --local-result-dir /builds/worker/artifacts/results --project moz-fenix --client-details matrixLabel=try failed with exit code 10
[task 2024-05-21T08:18:27.778Z] Copying gs://fenix_test_artifacts/2024-05-21_08-02-24.202283_KfpO/matrix_0/x1q-29-en_US-portrait/data_app_crash_0_org_mozilla_fenix_debug.txt to /builds/worker/artifacts/results
[task 2024-05-21T08:18:27.778Z] Copying gs://fenix_test_artifacts/2024-05-21_08-02-24.202283_KfpO/matrix_0/java-30-en_US-portrait/data_app_crash_0_org_mozilla_fenix_debug.txt to /builds/worker/artifacts/results
[task 2024-05-21T08:18:27.778Z] Copying gs://fenix_test_artifacts/2024-05-21_08-02-24.202283_KfpO/matrix_0/oriole-31-en_US-portrait/data_app_crash_0_org_mozilla_fenix_debug.txt to /builds/worker/artifacts/results
[task 2024-05-21T08:18:27.778Z] PROCESS-CRASH | org.mozilla.fenix.debug | java.lang.IllegalStateException: Composed into the View which doesn't propagate ViewTreeLifecycleOwner! at androidx.compose.ui.platform.AndroidComposeView.onAttachedToWindow(AndroidComposeView.android.kt:1347)
[task 2024-05-21T08:18:27.778Z] 	at android.view.View.dispatchAttachedToWindow(View.java:20753)
[task 2024-05-21T08:18:27.778Z] 	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3490)
[task 2024-05-21T08:18:27.778Z] 	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3497)
[task 2024-05-21T08:18:27.778Z] 	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2630)
[task 2024-05-21T08:18:27.778Z] 	at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2143)
[task 2024-05-21T08:18:27.778Z] 	at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8665)
[task 2024-05-21T08:18:27.778Z] 	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1037)
[task 2024-05-21T08:18:27.778Z] 	at android.view.Choreographer.doCallbacks(Choreographer.java:845)
[task 2024-05-21T08:18:27.778Z] 	at android.view.Choreographer.doFrame(Choreographer.java:780)
[task 2024-05-21T08:18:27.778Z] 	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022)
[task 2024-05-21T08:18:27.778Z] 	at android.os.Handler.handleCallback(Handler.java:938)
[task 2024-05-21T08:18:27.778Z] 	at android.os.Handler.dispatchMessage(Handler.java:99)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.base.Interrogator.loopAndInterrogate(Interrogator.java:10)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.base.UiControllerImpl.loopUntil(UiControllerImpl.java:7)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.base.UiControllerImpl.loopUntil(UiControllerImpl.java:1)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.base.UiControllerImpl.injectMotionEvent(UiControllerImpl.java:5)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.action.MotionEvents.sendUp(MotionEvents.java:6)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.action.MotionEvents.sendUp(MotionEvents.java:1)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.action.Tap.sendSingleTap(Tap.java:5)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.action.Tap.-$$Nest$smsendSingleTap(Unknown Source:0)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.action.Tap$1.sendTap(Tap.java:1)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.action.GeneralClickAction.perform(GeneralClickAction.java:4)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.ViewInteraction$SingleExecutionViewAction.perform(ViewInteraction.java:2)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.ViewInteraction.doPerform(ViewInteraction.java:23)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.ViewInteraction.-$$Nest$mdoPerform(Unknown Source:0)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.ViewInteraction$1.call(ViewInteraction.java:6)
[task 2024-05-21T08:18:27.778Z] 	at androidx.test.espresso.ViewInteraction$1.call(ViewInteraction.java:1)
[task 2024-05-21T08:18:27.778Z] 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[task 2024-05-21T08:18:27.778Z] 	at android.os.Handler.handleCallback(Handler.java:938)
[task 2024-05-21T08:18:27.778Z] 	at android.os.Handler.dispatchMessage(Handler.java:99)
[task 2024-05-21T08:18:27.778Z] 	at android.os.Looper.loopOnce(Looper.java:201)
[task 2024-05-21T08:18:27.778Z] 	at android.os.Looper.loop(Looper.java:288)
[task 2024-05-21T08:18:27.778Z] 	at android.app.ActivityThread.main(ActivityThread.java:7839)
[task 2024-05-21T08:18:27.778Z] 	at java.lang.reflect.Method.invoke(Native Method)
[task 2024-05-21T08:18:27.778Z] 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
[task 2024-05-21T08:18:27.778Z] 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
[task 2024-05-21T08:18:27.778Z] 
[task 2024-05-21T08:18:27.778Z] Command ./automation/taskcluster/androidTest/copy-robo-crash-artifacts.py failed with exit code 1
[taskcluster 2024-05-21 08:18:28.102Z] === Task Finished ===
[taskcluster 2024-05-21 08:18:29.832Z] Unsuccessful task run with exit code: 10 completed in 1208.619 seconds

Comment 8

[Aaron Train [:aaronmt]]

I filed each varying crash signature separately above, maybe both the same issue with slightly different stacks.

Primarily: java.lang.IllegalStateException: ViewTreeLifecycleOwner not found from mozilla.components.compose.cfr.CFRPopupFullscreenLayout{516aa61 V.E...... ......I. 0,0-720,1466}

Secondary: java.lang.IllegalStateException: Composed into the View which doesn't propagate ViewTreeLifecycleOwner!

This matrix has both signatures: https://console.firebase.google.com/project/moz-fenix/testlab/histories/bh.66b7091e15d53d45/matrices/7584839789056433591

Comment 9

[twhite]

Thanks Aaron, I'll look into this

Comment 10

[twhite]

QA can this be tested against all existing CFRs please with and without the keyboard being displayed.

• Private mode
• Jump back in
• Sync tab
• Shopping
• Inactive tabs
• Open in app

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/448e31e24729
Allow CFRs to be dismissed by outside touches. Interim fix to improve the current CFR implementation r=android-reviewers,vdreghici

Comment 12

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/448e31e24729

Comment 13

[Aaron Train [:aaronmt]]

Tom, possible crash regression happening in Robo-tests , in a similar ViewTreeLifecycleOwner related crash: bug https://bugzilla.mozilla.org/show_bug.cgi?id=1901940

Comment 14

[Lorand Janos]

Hi,
As I understood, the jump back in and the sync CFR behaviors are recently changed to not to be triggered when the keyboard is activated.
Inactive tabs CFR and open in app CFR cannot be tested with the keyboard displayed. Also, I couldn't test the shopping CFR, as it is no longer active.

I found the following issues:

1. On the private mode CFR, tapping outside of the CFR sends the app to the background, the CFR is displayed again on reopen, then it behaves the same way, until one of the option is selected from the CFR. Sometimes the app crashes when executing this test.
2. Inactive tabs CFR is only displayed after 14 days when the app remains open while the time is set, if I close the app and reopen it after I set the time 14 days ahead, the CFR is not displayed.
3. Open in app CFR is not dismissed by tapping outside of the CFR

Tested devices:

• Samsung Galaxy S23 Ultra (Android 14);
• Redmi Pad SE (Android 12).

Tested builds:

• Firefox Nightly 128.0a1 from 2024-06-14;
• Firefox Nightly 128.0a1 from 2024-06-17.

Please let me know if new tickets should be opened for the mentioned issues or will the issues be resolved here after reopening.

Comment 15

[twhite]

(In reply to Lorand Janos from comment #14)

I found the following issues:

1. On the private mode CFR, tapping outside of the CFR sends the app to the background, the CFR is displayed again on reopen, then it behaves the same way, until one of the option is selected from the CFR. Sometimes the app crashes when executing this test.
2. Inactive tabs CFR is only displayed after 14 days when the app remains open while the time is set, if I close the app and reopen it after I set the time 14 days ahead, the CFR is not displayed.
3. Open in app CFR is not dismissed by tapping outside of the CFR

Thank you Lorand!

1. This should be fixed in the latest Nightly with the inclusion of the fix added in https://bugzilla.mozilla.org/show_bug.cgi?id=1893003
2. This sounds out of scope for this particualr patch however it may related to inactive tab work (007 does this sound familiar at all?)
3. So it looks like that particular 'CFR' is actually not a CFR implemented using this class. I'll into this in an additional bug.

Can we re-test this on the latest Nightly please, in particular issue number 1?

Comment 16

[Lorand Janos]

Hi Tom,

I can confirm that the fix mentioned on point 1 seems to have helped, the private mode CFR is now working as intended, with it being possible to dismiss by tapping outside of it. Tested on portrait and landscape mode.

• Device used: Samsung Galaxy S23 Ultra (Android 14).

• Tested build: Firefox Nightly 129.0a1 from 2024-06-26.

Comment 17

[Donal Meehan [:dmeehan]]

:twhite, are you planning on landing anything else under this or should this be resolved?

Comment 18

[twhite]

Thank you Lorand.

Donal, no plans to land anything else for this bug, will set as resolved, thanks.