2064177 - Remove unnecessary `viewBox` (so we go back to automagically getting preserveAspectRatio="none" behavior) in film-strip SVG assets

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Sam Foster [:sfoster] (he/him)]

Bug 2063751 and bug 2063903 identified an issue where frames in the "film-strip" animations can get mis-aligned due to a pixel-snapping issue with SVGs. That was observed in the recently-updated download animation, but we use a similar pattern in a few other places.

• browser/themes/shared/downloads/notification-start-animation.svg
• browser/themes/shared/icons/chevron-animation.svg
• browser/themes/shared/icons/success-animation.svg

I'm not sure what specifically changed to make the regression - some dimension changed to another dimension and that happened to snap poorly at a particular scaling setting? ISTM we might want to be pre-emptive here and add the preserveAspectRatio="none" attribute to these other images. Or would you rather just get the underlying platform fix in :emilio?

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Sam Foster [:sfoster] (he/him) from comment #0)

ISTM we might want to be pre-emptive here and add the preserveAspectRatio="none" attribute to these other images.

I think that makes sense.

Or would you rather just get the underlying platform fix in

I wouldn't be surprised if the platform fix ends up having unforeseen complications and/or regressions, so it's probably better to not have our frontend have a hard dependency on it.

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Sam Foster [:sfoster] (he/him) from comment #0)

I'm not sure what specifically changed to make the regression

In the case of bug 2063751 for the download animation, the relevant change was the addition of a viewBox, here:

-<svg xmlns="http://www.w3.org/2000/svg" width="540" height="20">
+<svg xmlns="http://www.w3.org/2000/svg" width="800" height="20" viewBox="0 0 800 20">

https://hg-edge.mozilla.org/integration/autoland/rev/135fa4853d96#l4.7

If an SVG image lacks a viewBox, Gecko automatically treats it as having preserveAspectRatio="none", when displaying it as an image. Whereas when you declare an explicit viewBox (particularly one that's different from the declared size, but even one that's the same as the declared size), you're opting in to a more subtle mechanism for how the SVG content gets laid out into the viewport-that-is-provided (i.e. the area that's made available to the whole image, which you're slicing out a precise piece of).

So: any other SVG asset where we added a viewBox recently will likely want to get this same preserveAspectRatio="none" treatment, to avoid changing behavior and hitting similar bugs to bug 2063751.

Comment 3

[BugBot [:suhaib / :marco]]

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

Comment 4

[Daniel Holbert [:dholbert]]

One other equivalent simplification: if these images are all just being used as images (i.e. we're not having the user view them directly in a tab), we can just drop the viewBox attribute.

Specifically for SVG-as-an-image: when we see <svg width="someWidth" height="someHeight"> (with unspecified viewBox and preserveAspectRatio), all browsers automatically synthesize attributes viewBox="0 0 someWidth someHeight" preserveAspectRatio="none"` for that SVG element. But if viewBox and/or preserveAspectRatio are provided, then something more subtle will happen.

In this case it seems like we want those synthesized attribute values, so we can just drop the viewBox to get them automatically and to avoid repeating ourselves (repeating the SVG dimensions in the hardcoded viewBox).

So: if that's the behavior you want (and typically it is, if you're trying to make something that's analogous to a raster image but just scales more crisply), then the simplest approach is just to omit the viewBox and preserveAspectRatio attributes.

Comment 5

[Daniel Holbert [:dholbert]]

let me just take this since I'm already poking at it. :)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: Bug 2064177: Remove recently-added viewBox attrs from outer SVG element in SVG assets, so we can benefit from the automatically synthesized one (and get preserveAspectRatio="none" for free). r?sfoster

This viewBox attribute was unnecessary and slightly counterproductive, so
it's nicer to just remove it.

The explicit viewBox was unnecessary because Gecko will automatically
synthesize an appropriate viewBox (if the attribute is missing) on the
outermost SVG element in an SVG image, using the SVG element's declared 'width'
and 'height' attributes. So, it's nice for conciseness to benefit from this
and avoid needing to manually repeat the width and height in two spots on the
same element.

The explicit viewBox was counter-productive because: if we omit it and instead
allow Gecko to synthesize a viewBox, Gecko will also automatically apply
preserveAspectRatio="none" by default, which is essentially always the
behavior we really want our SVG assets to have. On the other hand, if a viewBox
is explicitly provided on the outermost SVG element, then Gecko uses the SVG
spec's default preserveAspectRatio="xMidYMid" behavior, which is essentially
a "contain" constraint and can cause subly unexpected positioning glitches like
https://bugzilla.mozilla.org/show_bug.cgi?id=2063751 in cases where slight
pixel-rounding issues can cause the aspect ratio to be ever-so-slightly off
from the image's natural aspect ratio.

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Sam Foster [:sfoster] (he/him) from comment #0)

we use a similar pattern in a few other places.
[...]

• browser/themes/shared/icons/chevron-animation.svg

This one^ didn't get a viewBox attribute added to it:
https://searchfox.org/firefox-main/rev/b462c13f11417e13461f1202d71b14e2784f5db0/browser/themes/shared/icons/chevron-animation.svg#4

<svg xmlns="http://www.w3.org/2000/svg" width="848" height="16">

So it doesn't need editing.

The other ones (and the ones from bug 2063751) do, though.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/393b66709162
https://hg.mozilla.org/integration/autoland/rev/a1a22b0464d2
Remove recently-added viewBox attrs from outer SVG element in SVG assets, so we can benefit from the automatically synthesized one (and get preserveAspectRatio="none" for free). r=sfoster,desktop-theme-reviewers

Comment 9

[Cristian Tuns]

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

Comment 10

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:dholbert, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox155 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Daniel Holbert [:dholbert]]

Yes - the issue described in bug 2063751 is definitely important enough to require an uplift, and the patch here simplifies our fix there and also makes the same fix to some other files just in case they might hit the same problem.

Comment 12

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: This patch stack fixes a bug that can cause "filmstrip-style" animations (e.g. download button) to render with a broken-looking sideways jitter.
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: yes
• Steps to reproduce for manual QE testing: 1. In Ubuntu system settings app, "displays" section, choose "scale: 166%"

2. Launch Firefox.
3. Download a file, e.g. one of the downloads from https://www.firefox.com/en-US/thanks/
4. Watch the download icon on your Firefox toolbar carefully, as it animates to show download progress and then the completed download.

actual: sideways jittering/slippage (video in https://bug2063751.bmoattachments.org/attachment.cgi?id=9626911 )
expected: no sideway jittering/slippage

• Risk associated with taking this patch: low
• Explanation of risk level: Targeted fix, removing an SVG attribute that was added in a recent refresh of these assets but was unnecessary and had unforeseen side effects.
• String changes made/needed?: None
• Is Android affected?: no

Comment 13

[Daniel Holbert [:dholbert]]

Attachment: Bug 2064177: Remove recently-added viewBox attrs from outer SVG element in SVG assets, so we can benefit from the automatically synthesized one (and get preserveAspectRatio="none" for free).

This viewBox attribute was unnecessary and slightly counterproductive, so
it's nicer to just remove it.

The explicit viewBox was unnecessary because Gecko will automatically
synthesize an appropriate viewBox (if the attribute is missing) on the
outermost SVG element in an SVG image, using the SVG element's declared 'width'
and 'height' attributes. So, it's nice for conciseness to benefit from this
and avoid needing to manually repeat the width and height in two spots on the
same element.

The explicit viewBox was counter-productive because: if we omit it and instead
allow Gecko to synthesize a viewBox, Gecko will also automatically apply
preserveAspectRatio="none" by default, which is essentially always the
behavior we really want our SVG assets to have. On the other hand, if a viewBox
is explicitly provided on the outermost SVG element, then Gecko uses the SVG
spec's default preserveAspectRatio="xMidYMid" behavior, which is essentially
a "contain" constraint and can cause subly unexpected positioning glitches like
https://bugzilla.mozilla.org/show_bug.cgi?id=2063751 in cases where slight
pixel-rounding issues can cause the aspect ratio to be ever-so-slightly off
from the image's natural aspect ratio.

Original Revision: https://phabricator.services.mozilla.com/D319252

Comment 14

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/d1d973c6bd54
https://hg.mozilla.org/releases/mozilla-beta/rev/2ed8a5189cd6

Comment 15

[Raluca Popovici, Desktop QA]

I was able to reproduce this issue in Firefox Beta 154.0b1 using Ubuntu 24.10.
The issue is fixed in the Firefox Beta treeherder build 155.0b2 and the latest Nightly 156.0a1 (2026-08-19) using Ubuntu 24.10, macOS 13.2.1 and Windows 10.