1888657 - Useless CSS animation is running while the viewer is doing nothing

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect, P1)

Description

[Calixte Denizet (:calixte)]

STR:

• open a pdf
• wait a little and take a profile

Actual result:
In the marker chart we can see that a CSS animation is running (named spinLoadingIcon).

Expected result:
Nothing is running.

Here's a profile:
https://share.firefox.dev/3vtsL0X

Comment 1

[BMO Github Automation]

Attachment: [mozilla/pdf.js] Avoid useless CSS animation when nothing is done (bug 1888657) (#17855)

Comment 2

[Calixte Denizet (:calixte)]

:emilio, I'm wondering if more generally it could be a bug with the animation in the SVG image, I mean once the icon has its visibility set to hidden, then the animation in the SVG should be cancelled, wdyt ?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Visibility: hidden doesn't cancel animations per spec (because visibility: visible descendants may be visible and depend on the animation, yes, it sucks). But we should be able to throttle them and I think they're throttled in this case.

Comment 5

[Florian Quèze [:florian]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

Visibility: hidden doesn't cancel animations per spec (because visibility: visible descendants may be visible and depend on the animation, yes, it sucks). But we should be able to throttle them and I think they're throttled in this case.

Could we detect that there's no visibility: visible descendants and cancel the animation anyway?

For every tick, there are 2 "RefreshDriverTick" markers, one with "Tick reasons: HasObservers (DocumentTimeline animations [Style])" (that's the CSS animation) and one with "Tick reasons: HasImageAnimations". Do we know which image is being animated here?

Comment 6

[Calixte Denizet (:calixte)]

The image being animated is:
https://github.com/mozilla/pdf.js/blob/master/web/images/loading.svg

Comment 7

[Calixte Denizet (:calixte)]

It's a regression from bug 1866850 and especially from:
https://github.com/mozilla/pdf.js/pull/17287

Comment 8

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

Set release status flags based on info from the regressing bug 1866850

Comment 9

[Calixte Denizet (:calixte)]

Attachment: Bug 1888657 - Avoid useless CSS animation when nothing is done in the pdf viewer

Comment 10

[Phabricator Automation]

release Uplift Approval Request

• User impact if declined: Some power will be uselessly used
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: See STR in comment#0
• Risk associated with taking this patch: Low
• Explanation of risk level: Self contained and very small change.
• String changes made/needed: No
• Is Android affected?: no

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/10f793276d1a

Comment 12

[Negritas Sergiu, Desktop QA]

Hello! I have managed to reproduce the issue with firefox 124.0.2 on Ubuntu 22.04, I can confirm that the issue is fixed with firefox 125.0 and 126.0a1(2024-04-08) on Ubuntu 22.04, MacOS 12.6 and Windows 10. I will update the status and flags of this bug and also will remove the qe-verify+ marker.

Have a nice day!