Android settings UI does not respect animation settings [372605006]

Assigned

Bug

Needs-Bisect

Status Update

No update yet.

Description

wf...@chromium.org

created issue #1

Oct 10, 2024 05:38PM

Application Version: 131.0.6768.0 canary
Android Build Number: Android 15 Pixel 7 Build AP3A.241005.015; 35; REL
Device: Pixel 7

Steps to reproduce:

disable system animations in android settings.
go to settings
click on a setting e.g. 'privacy and security' or any of them

Observed behavior:

There is an animation - it seems to slide in from the right.

Expected behavior:

No animation

Frequency:
every time

Additional comments:

This seems like a regression since I only noticed this a few days ago on Canary.

Comments

wf...@chromium.org <wf...@chromium.org> #2Oct 10, 2024 05:42PM

This bug been identified as part of Chrome's flake reduction initiative. Resolution of this issue is critical because it has a large impact on the stability of Chrome CI/CQ. Resolving these issues will be essential to meet Chrome’s flake reduction goals.

This bug has led to a large number of gardened builds failing in the last 7 days. List of gardened build failures caused by this cluster can be viewed at:

https://data.corp.google.com/sites/chrome_generic_flakiness_dashboard_datasite/build_failures/?av=rpqxzw:ktu56&fb=is_sheriff_or_cq_builder:eq:true&f=cluster_name:in:b%2F400617105
or in LUCI Analysis and grouped by builders:

https://luci-milo.appspot.com/ui/tests/p/chromium/rules/3519deb73451d0c9b91241ccea2406fa?tab=recent-failures&filterToMetric=builds-failed-due-to-flaky-tests&groupBy=builder&orderBy=invocationFailures

Please consider the following strategies to mitigate the impact from this issue which are rated in order of resolution preference:
1. Resolve the underlying test issue.
2. Move the flaky test from Critical Builders to FYI Builder
3. Disable test (least desirable as it reduces test coverage) and add a Test-Disabled label to this issue. The disabled tests might continue running in reviver builders (go/test-reviver), see config [1] for a list of supported builders.

http://go/resolve-chrome-top-flakes provides more information on the tools available for resolving flaky tests.

When investigating this failure, you may identify this bug is too broad (encompasses multiple different issues) or too narrow (only captures one part of a larger issue). If this applies, you can combine issues[2] or split issues[3].

Links:
[1]

https://source.chromium.org/chromium/chromium/src/+/main:infra/config/subprojects/reviver/reviver.star
[2]

https://luci-analysis.appspot.com/help#combining-issues
[3]

https://luci-analysis.appspot.com/help#splitting-issues

Why LUCI Analysis posted this comment:

https://luci-milo.appspot.com/ui/tests/help#policy-activated (Policy ID: builds-failed-due-to-flaky-tests)

wf...@chromium.org <wf...@chromium.org> #3Oct 10, 2024 05:43PM

reproduces on 131.0.6752.0 dev

wf...@chromium.org <wf...@chromium.org> #4Oct 10, 2024 09:08PM

I see some commits from nya@ here, so maybe they know what might have changed here.

ny...@google.com <ny...@google.com> #5Oct 11, 2024 03:14AM

Accepted by ny...@google.com.

Yeah it's likely it's caused by my change. Let me take a look.

ny...@google.com <ny...@google.com> #6Oct 11, 2024 07:09AM

Hmm, I searched the web for a while but I couldn't find a reliable way to make fragment custom animations aware of system animation settings. The only way I could find is to explicitly check setting values such as ANIMATOR_DURATION_SCALE and apply it somehow. Does anybody have better idea?

tw...@google.com <tw...@google.com> #7Oct 14, 2024 06:45PM

It's surprising that the custom transition's, which are using Android APIs, don't already automatically account for this. I'd expect any transitions we set through the system to respect Android settings... seems worth raising a bug with Android or reaching out to android-chatty-eng@ to see if anyone has tips.

There is some precedent for using ANIMATOR_DURATION_SCALE for compositor (i.e. C++) animations: https://source.chromium.org/search?q=ANIMATOR_DURATION_SCALE&ss=chromium

ny...@google.com <ny...@google.com> #8Oct 15, 2024 01:38AM

Accepted by ny...@chromium.org.

Theresa: Sounds good to check with android-chatty-eng@. I've sent a message and included you in CC.

ny...@google.com <ny...@google.com> #9Oct 17, 2024 03:19AM

Got a reply from ilake@:

There are two types of transitions setCustomAnimations() support - the API 1 Animation class (R.anim) and the API 11 Animator class (R.animator).

Only Animator (and APIs built on top of it, like Transition) respect the user's animation settings. That, and other reasons like Predictive Back which are only supported by Animator, are why you should strongly consider only using Animator APIs.

I'm sending a patch to convert the animations to property animations.

ap...@google.com <ap...@google.com> #10Oct 22, 2024 07:50AM

Project: chromium/src
Branch: main
Author: Shuhei Takahashi <nya@chromium.org>
Link: https://chromium-review.googlesource.com/5939236

[Settings] Convert material animations to property animations

Expand for full commit details

[Settings] Convert material animations to property animations

It turned out that tween animations (in res/anim/) do not consider
accessibility settings. This patch converts material animations to
property animations (in res/animator/) so that they honor settings.

Video:
http://go/scrcast/NjMzNDYyNzM3ODYyNjU2MHxhODE1Mjg3Mi0zNQ

Bug: 372605006
Change-Id: I64547aaa7be4cb56b517cc5f36d2e710aa5d3eed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5939236
Commit-Queue: Shuhei Takahashi <nya@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1371882}

Files:

M chrome/android/java/src/org/chromium/chrome/browser/settings/SettingsActivity.java
M components/browser_ui/styles/android/BUILD.gn
D components/browser_ui/styles/android/java/res/anim/shared_x_axis_close_enter.xml
D components/browser_ui/styles/android/java/res/anim/shared_x_axis_close_exit.xml
D components/browser_ui/styles/android/java/res/anim/shared_x_axis_open_enter.xml
D components/browser_ui/styles/android/java/res/anim/shared_x_axis_open_exit.xml
A components/browser_ui/styles/android/java/res/animator/shared_x_axis_close_enter.xml
A components/browser_ui/styles/android/java/res/animator/shared_x_axis_close_exit.xml
A components/browser_ui/styles/android/java/res/animator/shared_x_axis_open_enter.xml
A components/browser_ui/styles/android/java/res/animator/shared_x_axis_open_exit.xml

Hash: 9f8a28f9592da5af2b7926b87fcc7fba3235de87
Date: Tue Oct 22 07:46:37 2024

ny...@google.com <ny...@google.com> Oct 23, 2024 02:16AM

Marked as fixed.

pe...@google.com <pe...@google.com> #11Oct 23, 2024 02:27AM

*This merge request uses Chrome's new merge process. Find more information at go/chrome-merge-quickstart.

M131 merge request created.

Please update crbug/375074988 to have this merge reviewed.

ny...@google.com <ny...@google.com> #12Oct 25, 2024 05:13AM

Status: Assigned (reopened)

Unfortunately I'm reverting the patch due to its side-effect bug (crbug.com/375159749).

ap...@google.com <ap...@google.com> #13Oct 25, 2024 02:40PM

Project: chromium/src
Branch: main
Author: Shuhei Takahashi <nya@chromium.org>
Link: https://chromium-review.googlesource.com/5964636

Revert "[Settings] Convert material animations to property animations"

Expand for full commit details

Revert "[Settings] Convert material animations to property animations" 
 
This reverts commit 9f8a28f9592da5af2b7926b87fcc7fba3235de87. 
 
Reason for revert: Turned out to cause visual glitches: crbug.com/375159749 
 
Original change's description: 
> [Settings] Convert material animations to property animations 
> 
> It turned out that tween animations (in res/anim/) do not consider 
> accessibility settings. This patch converts material animations to 
> property animations (in res/animator/) so that they honor settings. 
> 
> Video: 
> http://go/scrcast/NjMzNDYyNzM3ODYyNjU2MHxhODE1Mjg3Mi0zNQ 
> 
> Bug: 372605006 
> Change-Id: I64547aaa7be4cb56b517cc5f36d2e710aa5d3eed 
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5939236 
> Commit-Queue: Shuhei Takahashi <nya@chromium.org> 
> Reviewed-by: Wenyu Fu <wenyufu@chromium.org> 
> Cr-Commit-Position: refs/heads/main@{#1371882} 
 
Bug: 372605006, 375159749 
Change-Id: If6e23bac3eb7d21078959515b9c2ff3fcbbb9536 
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5964636 
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> 
Reviewed-by: Theresa Sullivan <twellington@chromium.org> 
Commit-Queue: Theresa Sullivan <twellington@chromium.org> 
Auto-Submit: Shuhei Takahashi <nya@chromium.org> 
Cr-Commit-Position: refs/heads/main@{#1373895}

Files:

M chrome/android/java/src/org/chromium/chrome/browser/settings/SettingsActivity.java
M components/browser_ui/styles/android/BUILD.gn
A components/browser_ui/styles/android/java/res/anim/shared_x_axis_close_enter.xml
A components/browser_ui/styles/android/java/res/anim/shared_x_axis_close_exit.xml
A components/browser_ui/styles/android/java/res/anim/shared_x_axis_open_enter.xml
A components/browser_ui/styles/android/java/res/anim/shared_x_axis_open_exit.xml
D components/browser_ui/styles/android/java/res/animator/shared_x_axis_close_enter.xml
D components/browser_ui/styles/android/java/res/animator/shared_x_axis_close_exit.xml
D components/browser_ui/styles/android/java/res/animator/shared_x_axis_open_enter.xml
D components/browser_ui/styles/android/java/res/animator/shared_x_axis_open_exit.xml

Hash: 80b94fedffa4609fc645fc23cf2be6f9a98e5670
Date: Fri Oct 25 14:37:13 2024

IssueTracker

This issue is a part of the Chromium tracker.