1812063 - Animations Accessibility/Reduce Motion settings in Android not respected in Fenix

Status: RESOLVED FIXED

(Firefox for Android :: General, defect)

Description

[Christian Sadilek [:csadilek]]

From github: https://github.com/mozilla-mobile/fenix/issues/3597.

Steps to reproduce

1. Navigate to settings > accessibility
2. Enable "Remove animations"
3. Use Fenix

Expected behavior

Tab switching animation is disabled.

Actual behavior

Tab switching animation is not disabled.

Device information

• Android device: Pixel 2
• Fenix version: latest

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

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

The severity field is not set for this bug.
:cpeterson, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Segun Famisa [:segun]]

I was looking at this today, and want to explore some ideas to solve it.

I can confirm we do not respect the reduced motion and it's affecting every single place we define our own custom animation in our navigation graph xml, which is.....a lot

https://searchfox.org/firefox-main/search?q=Anim%3D%22&path=.xml&case=false&regexp=false

Comment 4

[Segun Famisa [:segun]]

Upgrading the severity to S2 because it's an accessibility issue and a workaround does not exist.

Comment 5

[Segun Famisa [:segun]]

Attachment: Bug 1812063 - Implement a compat NavHostFragment and custom Navigator that respects device reduced-motion setting

Comment 6

[Segun Famisa [:segun]]

This bug is happening because we have set custom animations with custom duration that we have defined in all our animations.

Some options I considered to solve the bug:

Option #1 - Use style attributes to set the duration

I considered using theme attributes to set the duration, instead of the hard-coded value, and then set the value in our Normal & Private tab themes.
This was considered too risky because we would need to ensure that every single theme sets this attribute default value. Else, we could end up with runtime fatalities

Option #2 - Use an extension function to programmatically check settings before navigation

This seems like a welcome approach, but involves changing a lot of files, and leaves room for missing some options. We will need to go through every place each "navigation action of interest" is being dispatched from.

Option #3 - Custom navigator alone

I tried using a custom Navigator to implement the system reduced motion awareness, but I encountered another problem.
We set the nav graph in the fragment container view directly in activity_home.xml.

Adding the custom Navigator and then programmatically setting the nav graph in HomeActivity by doing HomeActivity.navHost.navController.setGraph(..), causes the fragment transaction (that powers the navigation) to be queued up, and by the time HomeFragment gets created, the fragment may not have been set, and any findNavController() that happens in HomeFragment could throw. I experienced this crash a few times.

Option #4 - Custom navigator + custom nav host fragment

The working solution was to combine the custom navigator with a custom NavHostFragment that overrides the onCreateNavController(..) so we can set our custom navigator that is aware of motion.

---

Another option that could be explored, which I did not have time to explore was to keep our custom animations, but rely on the system animation duration. I do not have context about why we have those values, and I am not confident enough to change the values.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d5171e9e0f47
https://hg.mozilla.org/integration/autoland/rev/e3ade567c585
Implement a compat NavHostFragment and custom Navigator that respects device reduced-motion setting r=android-reviewers,kaya

Comment 8

[pstanciu]

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

Comment 9

[Segun Famisa [:segun]]

Nominating this for the next dot release because it's a rather important accessibility fix, and the blast radius for the fix is small.

Comment 10

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

Since the status is marked as fixed for nightly and as ? for release, is it fixed or ? for beta?
For more information, please visit BugBot documentation.