1916111 - SVG <polygon>/<polyline> fail to scale down from CSS 'zoom' (causing election map to be cut off at https://www.wahlen.sachsen.de/landtagswahl-2024-wahlergebnisse.php )

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: screenshot: map cut off

Firefox 131.0a1 20240901092144, 130.0 rc 1 and 129.0.2 on Windows 10 with 125% zoom on the OS level

The map at https://www.wahlen.sachsen.de/landtagswahl-2024-wahlergebnisse.php is cut off at the right and bottom. The page is right now under heavily load and a reload might be need to load it successfully, but this issue should disappear in the next two hours.

Robert, do you want to take a look at this?

Comment 1

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

:longsonr, since you are the author of the regressor, bug 1905023, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

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

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

Comment 3

[Robert Longson [:longsonr]]

The page looks as it should now as far as I can tell. I expect it was coded against the zoom bug(s) Firefox used to have.

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The issue still reproduces here. But yes, removing the zoom: .9; from .svg-map fixes the issue.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (partially reduced)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (further reduced)

This testcase demonstrates that this is an issue with specifically the SVG polygon element, which (unlike other shapes) fails to scale down with CSS zoom for some reason.

Comment 7

[Daniel Holbert [:dholbert]]

Using testcase 2, I can reproduce this as far back as Nightly 2023-09-29 if I manually enable layout.css.zoom.enabled. (That's the first Nightly where that pref existed, from bug 1854441.)

So I think this is just a bug in our CSS zoom impl, where it doesn't interact nicely with SVG <polygon>-in-particular for some reason.

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (more-severe zoom factor, with viewBox absent vs present)

Comment 9

[Daniel Holbert [:dholbert]]

Summing up the situation here -- the map on the live site here happens to use zoom, polygon, and viewBox, which means it's susceptible to two bugs, which happened to cancel each other out, sort of. And we fixed one, which is why the issue is now visible and why it manifests as a regression from bug 1905023 (per comment 1) even though really it's not indicative of a problem in that bug's patch (I think).

Specifically:
(A) we have a bug where we *fail to apply zoom to polygon. (This bug persists in current Nightly, and it's what testcases 2 and 3 reveal; and it goes back to our initial zoom impl in bug 1854441 as noted in comment 7, and I think it's the defect that we should focus on here.)
(B) until recently we had a bug where we'd redundantly apply the zoom to the SVG-viewport-as-a-whole (or something along those lines) if a viewBox attribute is present. That issue was bug 1905023.

On this particular site, issue (A) was essentially getting papered over by issue (B). When we fixed issue (B) (in bug 1905023), issue (A) became visible.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: screenshot showing how things have changed (left 3 are Nightlies, right is Chrome)

Here's a screenshot showing how our rendering has evolved on testcase 3. From left-to-right, the browsers in this screenshot are as follows:

• #1 is Firefox Nightly before we landed support for CSS zoom.
• #2 is Firefox Nightly 2023-09-29 just after we landed support for CSS zoom (manually preffed on)
• #3 is Firefox Nightly 2024-09-04 (today)
• #4 is Chrome

bug 1905023's fix is responsible for the difference in the lower portion of browsers #2 and #3 in the screenshot (which is the section with a viewBox). In browser #2, the polygon (the red shape) happens to end up the right size, though the lime rect is sized wrong -- this is what I mean by "issue (A) was essentially getting papered over by issue (B)" in my previous coment.

Comment 11

[Daniel Holbert [:dholbert]]

paraphrasing some notes from longsonr in chat: we fixed zoom for <path> (the blue line in testcase 3) in bug 1903361, and we did shapes except polygons via bug 1899695 and bug 1903352, because everything except poly[gon/line] is an SVGLength.

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: testcase 4: same as 3 but now with polyline

Here's a testcase with polyline for completeness. It's broken just like polygon is, per previous comment.

Comment 13

[Robert Longson [:longsonr]]

Attachment: Bug 1916111 - zoom polygons and polylines r=emilio

Comment 14

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=d76cca6c9c5d2e6ba26b99b935ee8a2f6dde3aaf

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74045e7e11c7
zoom polygons and polylines r=emilio

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47976 for changes under testing/web-platform/tests

Comment 17

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/74045e7e11c7

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 19

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

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox131 to wontfix.

For more information, please visit BugBot documentation.

Comment 20

[Robert Longson [:longsonr]]

Comment on attachment 9422686 [details]
Bug 1916111 - zoom polygons and polylines r=emilio

Beta/Release Uplift Approval Request

• User impact if declined: The website this bug was created to address doesn't work properly. Not sure how many people rely on it. Basically if you use CSS Zoom then SVG drawings are unlikely to render correctly.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We've already written these changes for lines, circles, ellipses etc. We forgot polygons and polylines when we did that so this is the same kind of change that we understood then extended to address the missing two shapes.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 21

[Dianna Smith [:diannaS]]

Comment on attachment 9422686 [details]
Bug 1916111 - zoom polygons and polylines r=emilio

Approved for 131.0b4

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/8650bfec19cc

Comment 23

[Ryan VanderMeulen [:RyanVM]]

This is probably worth an ESR128 uplift request also. Please nominate if you agree, it grafts cleanly.

Comment 24

[Robert Longson [:longsonr]]

Comment on attachment 9422686 [details]
Bug 1916111 - zoom polygons and polylines r=emilio

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: See user impact. This bug has the appearance of a regression even though it technically isn't. Before bug 1905023 we did two things wrong and in some cases (such as the affected website) they cancelled out. So yes sometimes two wrongs really do make a right.
• User impact if declined: The website this bug was created to address doesn't work properly. Not sure how many people rely on it. Basically if you use CSS Zoom then SVG drawings are unlikely to render correctly.
• Fix Landed on Version: 131
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): already in Firefox 131 without any reported issues.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9422686 [details]
Bug 1916111 - zoom polygons and polylines r=emilio

Approved for 128.4esr.

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/3ce142f981e4