Closed Bug 1910842 Opened 2 years ago Closed 2 years ago

Distortion while a checkbox animates on https://farbvelo.elastiq.ch/

Categories

(Core :: SVG, defect)

Product:
Core
Component:
SVG
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
131 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 --- fixed
firefox128 --- unaffected
firefox129 + wontfix
firefox130 + wontfix
firefox131 + fixed

People

(Reporter: mayankleoboy1, Assigned: longsonr)

References

(Regression,
URL
)

Details

(Keywords: regression)

Attachments

(4 files)

about:support
45.21 KB, text/plain
Details
bandicam 2024-07-31 12-57-27-823.mp4
1.56 MB, video/mp4
Details
Bug 1910842 - fix non-scaling-stroke when outer svg element has a CSS transform r=emilio
48 bytes, text/x-phabricator-request
Details | Review
Bug 1910842 - fix non-scaling-stroke when outer svg element has a CSS transform r=emilio
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-esr128+
Details | Review

Go to https://farbvelo.elastiq.ch/
Click on the gear icon on the bottom
A settings pane will open.
Scroll down a bit on this pane. There will be some options with checkboxes
Click on a checkbox

AR: While the checkbox animates, there is a distortion in the rendering
ER: Not so

Bisection: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2c8e99462d7dcbac2df0948cb9ab65d40d1d2fb7&tochange=256876c3862b6c4c8c51676f10d5ca440f86c0a8

Suspects: bug 1714763 and bug 1836693
cc :tnikkel

Attached file about:support —

I have a cached build of https://hg.mozilla.org/mozilla-central/rev/d588d40904f7880592d27c083cb64d31864340cb that I tested that was "good", so that probably means it was my bug in that range.

Interestingly, I can reproduce on nightly, but not on beta builds. Can you reproduce on any non-nightly build? I wonder if some differences on nightly that expose this?

I can reproduce the issue on Firefox129.0rc1 Windows11.
Click on the disabled checkbox Color Bleed. then black rectangle is rendered until mouse move over color bar.

Regression window in mozilla-beta:
https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=69f421f0aa2c1c8241d09309b4950a94ba2faec7&tochange=556215a082ccfb6f6b7c35c0e263e9fe585c318c

Regression window in mozilla-central:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e3cdd7bcd89ea80b3b400729e3a89b103e3c464c&tochange=a11e8418e383e89972a8546e6400f424b84a2ac5

Regressed by: Bug 1903546 - fix non-scaling-stroke

Regressed by: 1903546

Maybe there are 2 bugs here?

(In reply to Mayank Bansal from comment #5)

Maybe there are 2 bugs here?

That is what I am also thinking might be happening here.

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

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

For more information, please visit BugBot documentation.

status-firefox128: --- → unaffected
status-firefox129: --- → affected
status-firefox130: --- → affected
status-firefox-esr115: --- → unaffected
status-firefox-esr128: --- → unaffected
Flags: needinfo?(longsonr)

actually mozregression shows Firefox 115 is OK.

Flags: needinfo?(alice0775)

The first regression was caused by Bug 1714763 and Bug 1836693. It was then fixed by Bug 1898409.
However, a second regression was again caused by Bug 1903546.

(In reply to Robert Longson [:longsonr] from comment #9)

actually mozregression shows Firefox 115 is OK.

Yes, Firefox 115 is OK, works fine for me.

(In reply to Alice0775 White from comment #10)

The first regression was caused by Bug 1714763 and Bug 1836693. It was then fixed by Bug 1898409.
However, a second regression was again caused by Bug 1903546.

So sounds like the root cause is probably bug 1714763 then?

Assignee: nobody → longsonr
Pushed by longsonr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/daa273f24110 fix non-scaling-stroke when outer svg element has a CSS transform r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47433 for changes under testing/web-platform/tests

Backed out for causing dt failures on browser_inspector_highlighter-cssshape_offset-path.js

Backout link

Push with failures

Failure log

Flags: needinfo?(longsonr)

Can confirm that the autoland build fixes this bug for me.

Upstream PR was closed without merging

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

status-firefox131: --- → affected

Tentatively triaging as S2 given this is a recent regression that we shouldn't lose track of (particularly given that we've got a patch that was just backed out for regressing one automated test; hopefully it's not too hairy to untangle that).

Severity: -- → S2

Unfortunately the patch doesn't make us compatible with Chrome.

  1. Chrome (and Firefox without this patch) takes the outer svg viewBox into account with non-scaling-stroke. The current patch would exclude it.

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="-20 -20 50 50" width="100%" height="100%">
<rect width="10" height="10" stroke="lime" stroke-width="10" vector-effect="non-scaling-stroke" />
</svg>

The above SVG displays the same in Chrome and Firefox currently but would display all lime with the current patch.

What Chrome seems to do is ignore any transform on the outer SVG element.

Flags: needinfo?(longsonr)

Looks like that makes the existing test pass too.

Pushed by longsonr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/d568d18c7971 fix non-scaling-stroke when outer svg element has a CSS transform r=emilio

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

Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox131: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 131 Branch
Upstream PR merged by moz-wptsync-bot

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-firefox130 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(longsonr)
status-firefox130: affected → wontfix
Flags: needinfo?(longsonr)

I'm a bit confused about this being set to wontfix for 130 given it being rated S2 severity and tracking+ for Fx130. Is it your opinion that this bug isn't as severe as originally thought to be?

status-firefox129: affected → wontfix
Flags: needinfo?(longsonr)
Flags: in-testsuite+

I may've been off slightly by rating it S2; mostly wanted to be sure it didn't fall off the radar, since it was a regression with a nearly-landable patch.

Given that this bug was cosmetic (not site breaking), and we shipped the bug already in 129, and automated test coverage is somewhat lacking for non-scaling-stroke edge cases (meaning there's nonzero risk of regressions): I think wontfix for beta130 is the right call.

Severity: S2 → S3
Flags: needinfo?(longsonr)

We've not had any duplicates as far as I can tell. You need a non-scaling-stroke on a shape where there is an outer <svg> element that has a transform so it's not going to be that common. Until recently neither Chrome nor Safari supported transforms on <svg> elements so that's why such content is pretty rare.

Comment on attachment 9417380 [details]
Bug 1910842 - fix non-scaling-stroke when outer svg element has a CSS transform r=emilio

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: We need this to complete the non-scaling-stroke fixes for bug 1958768
  • User impact if declined: non-scaling-stroke is broken in some cases
  • Fix Landed on Version: 131
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): this fix landed on Firefox 131 so it's been in release for a while now.
Attachment #9417380 - Flags: approval-mozilla-esr128?
Attachment #9478965 - Flags: approval-mozilla-esr128?

esr128 Uplift Approval Request

  • User impact if declined: non-scaling-stroke is broken in some cases
  • Code covered by automated testing: yes
  • Fix verified in Nightly: yes
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: none
  • Risk associated with taking this patch: low
  • Explanation of risk level: been in non-ESR builds since Firefox 131
  • String changes made/needed: none
  • Is Android affected?: yes
Attachment #9417380 - Flags: approval-mozilla-esr128?
Attachment #9478965 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: