spicetify / cli

Command-line tool to customize Spotify client. Supports Windows, MacOS, and Linux.
https://spicetify.app
GNU Lesser General Public License v2.1
17.67k stars 706 forks source link

topbar mutation listener potentially causing freezes #2806

Closed ohitstom closed 5 months ago

ohitstom commented 5 months ago

🔍 Have you checked Spicetify.app page for your issue?

🔍 Is there already an issue for your problem?

ℹ Environment / Computer Info

Spotify for Windows (64 bit)
1.2.30.1135.g02fef27a
Spicetify v2.31.0
xpui_2024-01-29_1706560054673_02fef27
cef_120.2.3+g7d89c0c+chromium-120.0.6099.199
Runtime: Alloy

📝 Description

random spotify freezes that make dom completely non interactable and a bunch of reflow errors pointing to the same line in the wrapper, this specifically happens if i turn on spotify on the extensions page of marketplace and click the dropdown to navigate to the install page, i get an n^whatever on mousedown and my spotify freezes

📸 Screenshots

image image

rxri commented 5 months ago

Cannot reproduce on my end. I believe the issue lies in the re-rendering whole page/re-fetching the extensions/custom apps whenever you enter the sub-page in marketplace. The message/mousedown handlers are not used in MutationObserver. MutationObserver shows these "violations" because it's watching the DOM so it throws these warnings. Every freezes I experienced (going from lyrics-plus to search page) were resolved before merging this commit https://github.com/spicetify/spicetify-cli/commit/27f15b465bcda97eafb39119518d0ba3b8ec5697 (the fix I'm talking about: https://github.com/spicetify/spicetify-cli/pull/2801/commits/488118594d85e8511dfcd53d880ca22e3fab3215)


Spotify for Windows (64 bit)
1.2.30.1135.g02fef27a
Spicetify vDev
xpui_2024-01-29_1706560054673_02fef27
cef_120.2.3+g7d89c0c+chromium-120.0.6099.199
Runtime: Alloy
ohitstom commented 5 months ago

Cannot reproduce on my end. I believe the issue lies in the re-rendering whole page/re-fetching the extensions/custom apps whenever you enter the sub-page in marketplace. The message/mousedown handlers are not used in MutationObserver. MutationObserver shows these "violations" because it's watching the DOM so it throws these warnings. Every freezes I experienced (going from lyrics-plus to search page) were resolved before merging this commit https://github.com/spicetify/spicetify-cli/commit/27f15b465bcda97eafb39119518d0ba3b8ec5697 (the fix I'm talking about: https://github.com/spicetify/spicetify-cli/pull/2801/commits/488118594d85e8511dfcd53d880ca22e3fab3215)


Spotify for Windows (64 bit)
1.2.30.1135.g02fef27a
Spicetify vDev
xpui_2024-01-29_1706560054673_02fef27
cef_120.2.3+g7d89c0c+chromium-120.0.6099.199
Runtime: Alloy

this wasn't an issue until I updated Spotify from 1.2.29 but I also updated Spicetify at the same time, could this be a chromium issue or something?

its important to note I've replicated this issue on two computers in two households and it only seems to happen with spicetify installed...

rxri commented 5 months ago

make a video of the issue

ohitstom commented 5 months ago

make a video of the issue

can do when I get home 👍

edit: home pc has stopped doing it now, will keep an eye out though

Shadow-Devil commented 5 months ago

I also get random freezes on Windows 10 since some days. It all goes unresponsive for 10-20sek and then rebuilds the whole UI. After that, everything works again.

Edit: It doesn't happen inside the Marketplace but when generally browsing Spotify, like searching or clicking on a playlist etc.

ohitstom commented 5 months ago

I also get random freezes on Windows 10 since some days. It all goes unresponsive for 10-20sek and then rebuilds the whole UI. After that, everything works again.

Edit: It doesn't happen inside the Marketplace but when generally browsing Spotify, like searching or clicking on a playlist etc.

sounds like it's potentially a Spotify issue then, does it happen if you run spicetify restore and browse Spotify for a while?

Shadow-Devil commented 5 months ago

I will test it the next days

ohitstom commented 5 months ago

issue still isnt cropping up, might be worth leaving it a week then closing this issue - likely that this is just an issue with spotify itself

rxri commented 5 months ago

I was testing it, and the mutationObserver executes only once per page switch so that definitely not coming from this code

Shadow-Devil commented 5 months ago

I was using Spotify without Spicetify for a while now and had no freezes. Going to switch back using Spicetify.

ohitstom commented 5 months ago

I was using Spotify without Spicetify for a while now and had no freezes. Going to switch back using Spicetify.

let us know how it goes!

Shadow-Devil commented 5 months ago

Unfortunately, the issue started to appear again... I also updated to the newest version of the cli (2.31.2)

rxri commented 5 months ago

Report from Performance tab would be very useful in this case, since I seriously cannot reproduce it on my end

ohitstom commented 5 months ago

Report from Performance tab would be very useful in this case, since I seriously cannot reproduce it on my end

I believe the inspect element also locks up not sure how easy this would be

rxri commented 5 months ago

You can connect via your browser and it will not lock up

rxri commented 5 months ago

Anyone wanna test #2826? This should be much reliable fix and potentially fixes the freezes you all experienced

Shadow-Devil commented 5 months ago

I am really confused. I had Spicetify with devtools running yesterday and since then I don't experience any more freezes (even without devtools running). I'm currently unable to reproduce the issue.

rxri commented 5 months ago

You're on 1.2.31 or 1.2.30?

Shadow-Devil commented 5 months ago

Spotify for Windows (64 bit) 1.2.22.982.g794acc0a Spicetify v2.31.2 Theme: Comfy / Deep Extensions: songstats.js, spotifyGenres.js, theme.js Custom apps: marketplace

Shadow-Devil commented 5 months ago

I have updated now to 1.2.30

rxri commented 5 months ago

yea that was probably something on spicetify end anyway but if it was related to observer for topbar, that am not sure that's why I'm changing it regardless