Loading spinner SVG on yahoo.mydashboard.oath.com does not draw properly and causes memory leak/crash
Categories
(Web Compatibility :: Desktop, defect, P3)
Tracking
(firefox113 affected)
Tracking | Status | |
---|---|---|
firefox113 | --- | affected |
People
(Reporter: bbhtt.zn0i8, Unassigned)
References
(Depends on 3 open bugs, Blocks 1 open bug, )
Details
(Keywords: crash, hang, memory-leak)
Attachments
(4 files)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:110.0) Gecko/20100101 Firefox/110.0
Steps to reproduce:
Open https://yahoo.mydashboard.oath.com/ while logged in/logged out
See the spinner on the front page trying to load the privacy summary
Refresh the page and let it load
Interact with toolbar items ie. addon, profiler item hamburger menu etc.
Actual results:
Everything (addon, profiler item hamburger menu etc.) is frozen while that page loads
Expected results:
It should've opened the privacy summary instantly without any delayed spinner and without freeze. On Chromium 108.0.5359.124 the page loads normally without any delayed spinner or freeze.
I suspect the freeze has something to do with the spinner loading.
The issue seems to happen on both latest Fenix and Firefox Nightly. A screencap is attached.
I've attempted to capture a profile but because everything is frozen I can't open the profiler page or refresh/upload it. Firefox doesn't recover from the freeze, after a while system kills it (GNOME's "application not responding" screen appears).
Comment 2•2 years ago
|
||
The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.
Comment 3•2 years ago
|
||
Which distro do you run? Can you test with Wayland disabled (run with MOZ_ENABLE_WAYLAND=0).
Thanks.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #3)
Which distro do you run? Can you test with Wayland disabled (run with MOZ_ENABLE_WAYLAND=0).
Thanks.
Arch Linux GNOME 44 Wayland session.
Happens with MOZ_ENABLE_WAYLAND=0
as well
Tested with Nightly 113.0a1 (2023-03-29)
Thanks.
Comment 5•2 years ago
|
||
Please try to find a regression range (you get a pushlog url at the end):
$ pip3 install mozregression
$ ~/.local/bin/mozregression --good 2022-10-01 --bad 2023-03-31 -a https://yahoo.mydashboard.oath.com/
STR:
- On Ubuntu 22.10 visit https://yahoo.mydashboard.oath.com/.
- If a blue loading spinner does not appear, create a new tab and visit again.
Gets stuck on a broken loading animation. CPU is high, memory climbs in main process (about:memory
shows leak in heap-unclassified
) and browser becomes unresponsive. Sometimes it recovers but it can continue until crash.
Does not occur with svg.disabled = true
or when .loading-holder
is blocked. Saving a local copy of the loading SVG does not reproduce the issue but the animation is still broken, looks correct in Chrome.
SVG is only part of DOM during loading. Manually adding SVG after finished loading causes same issue. Manually adding SVG to a different site (eg wikipedia.org) does not reproduce issue.
Does not occur on Windows unless using debugger to pause while SVG is displayed or manually adding the SVG.
Not a recent regression, reproducible with 2020-01-01 build.
Performance profile: https://share.firefox.dev/3Zzfqgt
Comment 10•2 years ago
|
||
The severity field is not set for this bug.
:TYLin, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 11•2 years ago
|
||
From the profile in comment 9, I see almost every callstack containing CSPReportSenderRunnable
. Maybe this bug is related to CSP?
Comment 12•2 years ago
|
||
Tom, can you investigate a bit? We can reproduce but it's not fully clear what our options are this being an issue with the SVG <animate>
feature.
Can you take a look and explore this a bit, before we make a decision how to move forward?
Comment 13•2 years ago
|
||
This is a report only policy so we are not really blocking anything. The problem purely seems to be the totally massive amount of CSP reports that we are trying to send. We had a very similar bug 1804871 before and bug 1657519, which shows that Chrome coalesces reports together.
I think for this bug we should simply stop sending reports after we reached a fixed number of reports for a specific context. I am not sure what a good number would be. The Yahoo page seems to produce about 200 reports and for example stopping after 20 reports makes everything load.
Edit: A higher number 100 also seems to work. The important part seems to be that we stop at some point so what we can actually make progress and finish loading the page.
Comment 14•2 years ago
|
||
We could consider it a dupe of our "coalesce CSP reports" bug, but does Chrome issue reports for this, or block things if it's not a report-only policy? At first glance I didn't see any literal style attribute setting: is it the svg transforms that are being interpreted as inline style? Is that per spec, or just an implementation detail of how we mapped SVG transforms into CSS transforms internally?
Comment 15•2 years ago
|
||
re-adding the needinfo for Tom since comment 14 was more what Freddy was asking. We know the reporter's perf problem/hang is from the massive # of reports (a known issue, as you point out) but weren't sure if these are valid violations to begin with.
If the reports are legitimate we should make this "depend on" one of the existing bugs and move this specific bug over to Webcompat to see if we can reach out to Yahoo about changing their page or CSP to work around the problem, or maybe shim it with the Webcompat extension.
Comment 16•2 years ago
|
||
If we do move this back to webcompat it would be good to know what we could even do to shim this, short of altering their CSP header in some way (and would we want to do that)? We could try to reach out, but odds are that it would be best to match whatever Chrome is doing here, or we'll probably just run into it again some time (sites apparently are using CSP and other reporting APIs more and more).
Comment 17•2 years ago
|
||
The difference between Chrome and Firefox in regards to blocking inline styles caused by <animate> via CSP is covered by bug 1459872.
Comment 18•2 years ago
|
||
The severity field is not set for this bug.
:freddy, could you have a look please?
For more information, please visit BugBot documentation.
Updated•2 years ago
|
Comment 19•1 years ago
|
||
If you save the SVG to a local file and view it without ANY content-security-policy involved at all it still looks terrible compared to webkit and blink browsers. Made my "file://" child process burn 5% CPU just trying to animate it badly. (On a mac).
It appears marginally more like it's supposed to if you load it as an <IMG> but it's still not right. I can at least see dots forming the entire circle, but the animation doesn't go around. To test, copy the following and load from a new window. No CSP involved:
data:text/html,<img src=https://bug1806276.bmoattachments.org/attachment.cgi?id=9326270>
We have the other bugs for "CSP related to SVG" and "Limit excessive CSP violation reports". I'm sending this over to layout as an SVG correctness issue.
Comment 20•1 years ago
|
||
The file is invalid because begin attributes that are < 1 and > -1 must begin with 0 or -0. Check out the console, it's full of errors.
Chrome's parsing is invalid per the SVG and SMIL specifications. I raised https://github.com/w3c/svgwg/issues/394 but it's had no traction. While it talks about dur, begin is just the same for parsing: https://www.w3.org/TR/SVG11/animate.html#ClockValueSyntax
So the display by Firefox is correct per the current specifications, although I would like to see the specification changed to allow this.
Do you want this to be a Web Compatibility bug? Or.a wayland memory leak bug? Or a CSP bug?
Whatever it is, it's not strictly an SVG bug.
Comment 21•1 years ago
|
||
(In reply to Robert Longson [:longsonr] from comment #20)
So the display by Firefox is correct per the current specifications, although I would like to see the specification changed to allow this.
Thanks, Robert. Looking at the spec issue, it sounds like nobody is opposed to the proposal, but it's just a question of defining the grammar in spec terms.
Given that, and given that we're the only ones with our current behavior, I'd suggest we should just go ahead and make the leading zero optional, independent of the spec status, to get the compat win and make sites like this render as-intended. The spec text can catch up when the svg spec editors are ready to update it, but we don't need to block our users' experience on the spec change being finalized, unless there's some substantial ambiguity around particular edge cases that we need to wait on clarification for or something like that.
Do you want this to be a Web Compatibility bug? Or.a wayland memory leak bug? Or a CSP bug?
I suspect doing outreach to get the SVG file updated might be more trouble than it's worth. It looks like we have bug 1459872 and bug 1657519 already filed for the CSP issues here, and I think the memory issues are related to that (from us aggressively trying to report CSP violations); that's not wayland-specific, per comment 4.
Having said that: the SVG leading-0 parsing issue is entirely unrelated to the more-concerning freeze/etc. that the user reported in comment 0, so probably we should refocus this bug on that issue that the user reported, and spin off a new bug for the SVG parsing issue.
So the issue with this bug here (the badness described in comment 0) is essentially some blocking happening per bug 1459872, which then results in zillions of reports which we fail to coalesce per bug 1657519. Presumably we could fix this issue by fixing either of those bugs?
Updated•1 years ago
|
Comment 22•1 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #21)
I'd suggest we should just go ahead and make the leading zero optional,
[...] probably we should refocus this bug on that issue that the user reported, and spin off a new bug for the SVG parsing issue.
I spun off bug 1834892 for that parsing issue resulting in the visual glitch in the animation that dveditz described.
Comment 23•1 years ago
|
||
(In reply to Robert Longson [:longsonr] from comment #20)
The file is invalid because begin attributes that are < 1 and > -1 must begin with 0 or -0. Check out the console, it's full of errors.
Chrome's parsing is invalid per the SVG and SMIL specifications. I raised [....] So the display by Firefox is correct per the current specifications, although I would like to see the specification changed to allow this.
Safari handles the file the same way as Chrome and all the other chromium browsers out there. "technically correct" is not helping us when the spec doesn't match reality :-(
Do you want this to be a Web Compatibility bug? Or.a wayland memory leak bug? Or a CSP bug?
It doesn't make sense as a CSP bug since the work would still be in the other two identified CSP bugs. I thought comment 4 ruled out wayland? Fixing the SVG parsing won't change the overwhelm of CSP animation errors so I guess it should be a web compat bug that depends on the code changes we'd like to make.
Whatever it is, it's not strictly an SVG bug.
True, but there was some kind of SVG webcompat issue involved in the display -- but now Daniel has filed bug 1834892 for that so we don't need to use this bug for that. So Webcompat it is.
Description
•