Bug 1809660: Fix for unwanted Homepage preferences animation when Pocket feature is enabled/disabled

[t-p-white]

Also removed unused fun isFirefoxDefaultBrowser() as a side effect.

Before fix:

Screen.Recording.2023-01-11.at.15.24.47.mov

After fix (Pocket disabled):

Screen.Recording.2023-01-11.at.16.38.36.mov

After fix (Pocket enabled):

Screen.Recording.2023-01-12.at.11.07.30.mov

Pull Request checklist

• Tests: This PR includes thorough tests or an explanation of why it does not
• Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
• Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

QA

• QA Needed

To download an APK when reviewing a PR (after all CI tasks finished running):

1. Click on Checks at the top of the PR page.
2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
3. Click on the build-debug task.
4. Click on View task in Taskcluster in the new DETAILS section.
5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

Fixes #1809660

[gabrielluong]

The description was slightly misleading to me since we didn't technically remove any animation code. We were just setting the preference to not be visible before it hits the code that toggles its visibility.

[gabrielluong]

I am wondering if this would have an opposite effect (instead of shrinking as a result of the pref hiding) for locales that should see this where the layout will now grow as the preference becomes visible. It's currently show only for US and CA locales https://searchfox.org/mozilla-mobile/source/fenix/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt#37.

Please test this out before landing.

[t-p-white]

@gabrielluong yes this is correct, good catch. I have moved the Preferences set up from onResume (Called when the fragment is visible to the user) to setPreferenceScreen (which is used to configure preferences prior to be being displayed)

[t-p-white]

The description was slightly misleading to me since we didn't technically remove any animation code. We were just setting the preference to not be visible before it hits the code that toggles its visibility.

@gabrielluong thanks for the feedback, I can see why it could be confusing. I've updated it

[gabrielluong]

We got some test failures as a result of the onResume changes.

2023-01-12T11:38:36.3881177Z   TEST: GIVEN the Pocket sponsored stories preference is true WHEN accessing settings THEN the setting for it is checked
2023-01-12T11:38:36.3882390Z     W/PackageParser: Unknown element under <manifest>: queries at /home/runner/work/fenix/fenix/app/build/intermediates/apk_for_local_test/debugUnitTest/apk-for-local-test.ap_ Binary XML file line #45
2023-01-12T11:38:36.7879671Z     D/LifecycleMonitor: Lifecycle status change: androidx.fragment.app.FragmentActivity@730ec8e9 in: PRE_ON_CREATE
2023-01-12T11:38:36.7880594Z     D/LifecycleMonitor: Lifecycle status change: androidx.fragment.app.FragmentActivity@730ec8e9 in: CREATED
2023-01-12T11:38:37.2883581Z   FAILURE
2023-01-12T11:38:37.3879929Z 
2023-01-12T11:38:37.3909643Z java.lang.AssertionError
2023-01-12T11:38:37.3910434Z 	at org.junit.Assert.fail(Assert.java:87)
2023-01-12T11:38:37.3912182Z 	at org.junit.Assert.assertTrue(Assert.java:42)
2023-01-12T11:38:37.3912682Z 	at org.junit.Assert.assertTrue(Assert.java:53)
2023-01-12T11:38:37.3913454Z 	at org.mozilla.fenix.settings.HomeSettingsFragmentTest.GIVEN the Pocket sponsored stories preference is true WHEN accessing settings THEN the setting for it is checked(HomeSettingsFragmentTest.kt:129)
2023-01-12T11:38:37.3914436Z 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
2023-01-12T11:38:37.3915518Z 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
2023-01-12T11:38:37.3916324Z 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
2023-01-12T11:38:37.3917048Z 	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
2023-01-12T11:38:37.3918049Z 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
2023-01-12T11:38:37.3918680Z 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
2023-01-12T11:38:37.3919281Z 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
2023-01-12T11:38:37.3919860Z 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
2023-01-12T11:38:37.3920416Z 	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
2023-01-12T11:38:37.3920971Z 	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
2023-01-12T11:38:37.3921484Z 	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
2023-01-12T11:38:37.3922015Z 	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:580)
2023-01-12T11:38:37.3922571Z 	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:287)
2023-01-12T11:38:37.3923111Z 	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:99)
2023-01-12T11:38:37.3923899Z 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
2023-01-12T11:38:37.3924409Z 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
2023-01-12T11:38:37.3924960Z 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
2023-01-12T11:38:37.3925385Z 	at java.base/java.lang.Thread.run(Thread.java:829)
2023-01-12T11:38:37.3925594Z 
2023-01-12T11:38:37.3925888Z   TEST: GIVEN the Pocket sponsored stories feature is disabled for the app WHEN accessing settings THEN the settings for it are not visible
2023-01-12T11:38:37.3926949Z     W/PackageParser: Unknown element under <manifest>: queries at /home/runner/work/fenix/fenix/app/build/intermediates/apk_for_local_test/debugUnitTest/apk-for-local-test.ap_ Binary XML file line #45
2023-01-12T11:38:37.6916453Z     D/LifecycleMonitor: Lifecycle status change: androidx.fragment.app.FragmentActivity@1151c96d in: PRE_ON_CREATE
2023-01-12T11:38:37.7879843Z     D/LifecycleMonitor: Lifecycle status change: androidx.fragment.app.FragmentActivity@1151c96d in: CREATED
2023-01-12T11:38:38.2886939Z   FAILURE

https://pipelines.actions.githubusercontent.com/serviceHosts/76ef84e6-2541-41e9-85c1-758085b4811e/_apis/pipelines/1/runs/116915/signedlogcontent/10?urlExpires=2023-01-12T16%3A27%3A51.4513812Z&urlSigningMethod=HMACV1&urlSignature=3afV75R317B159u7Cb1Z0xMLo6QVKIodcccnupS5dj4%3D

[gabrielluong]

I imagine we do this in other settings fragment so we would also encounter the same issue. Let's file a followup to also utilize setPreferenceScreen for the others after this lands and is QA verified.

[Mugurell]

A ticket for this issue already exists - #19051.

[t-p-white]

Thank you @Mugurell

[gabrielluong]

I do see in #19051 that @Mugurell recommended using onCreatePreferences vs our current approach here that uses setPreferenceScreen. I haven't had a chance to look up the difference between the two, but I am curious about the pros and cons about each method.

[t-p-white]

My initial implementation was to use onCreatePreferences func however I changed the implementation after reading this in the onCreatePreferences docs:

Called during onCreate(Bundle) to supply the preferences for this fragment. Subclasses are expected to call setPreferenceScreen(PreferenceScreen) either directly or via helper methods such as addPreferencesFromResource(int).

Which reading again now I think I may have misinterpreted.
Edit: I'm actually going to update to onCreatePreferences, no issues noted upon changing

[gabrielluong]

Could we also change the title and commit messages to say Bug 1809660? Using the existing "For #xxxx" convention would be more applicable for GitHub issues, but there's no #1809660 issue.