Bug 281920

Summary: Page freezes with many DOM manipulations and complex :has() selectors present
Product: WebKit Reporter: Roman Komarov <kizmarh>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Blocker CC: bfulgham, bramus, karlcow, koivisto, rik, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari 18   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
A screenshot of a page with clipped text, below is the top part of the Activity Monitor window that shows https://kizu.dev process with 100% CPU load. none

Roman Komarov
Reported 2024-10-22 14:24:42 PDT
Created attachment 473012 [details] A screenshot of a page with clipped text, below is the top part of the Activity Monitor window that shows https://kizu.dev process with 100% CPU load. I published a new article that explores some of the uses of `:has()` as well as potential cases for `sibling-index()` and `sibling-count()`. The article ended up being long, with many demos and code snippets. Here it is: https://kizu.dev/tree-counting-and-random/ Long story short: opening this page in Safari lead to a full page freeze, with CPU keeping on 100% for minutes. I pin-pointed the issue: There is syntax highlighting via Prism.js that happens when the page loads. Before syntax highlighting, the page contained 3125 elements (counting what document.querySelectorAll('*') matches). After Prism.js highlighting, where it adds all the tokens into the `<pre><code>` blocks, it bumps up to 6481 elements. Apparently, this was enough for Safari to freeze. For now I pushed a very quick fix to that disables the syntax highlighting only on this page, and only for browsers that do not support `requestIdleCallback` (which is, for now, enough for the stable Safari, so people could at least read the article; will probably later use a different detection method). Though, in Safari TP the page still can freeze on load (I guess, there is an experimental `requestIdleCallback` implementation?) In stable Safari, I am able to reproduce the freeze if I go to https://kizu.dev/tree-counting-and-random/, open console, and run console.time(); Prism.highlightAll(); console.timeEnd(); If the page was fully loaded, sometimes it works, but ends up taking ~3 seconds. In most other times, especially if you'd refresh the page with the console open and run it, the page freezes completely. My guess: each DOM change leads to full selectors list invalidation, which with the page using really complex `:has()` selectors, leads to the freeze. I'll try to find time to reproduce this outside of my site's article, but wanted to report this anyway today, so it would be on your radar. I'm putting the priority as “Blocker”, as it was, in fact, a blocker for my article to be readable (and still is), and I can imagine that it might be possible to create a malicious CSS that would contain just a few `:has()` selectors that could freeze any complex-enough website. I'm attaching a screenshot from Safari TP with the page frozen (can be seen by trimmed text), and an Activity Monitor opened showing 100% for my website. I managed to reproduce this on two different MacBooks, and one iPhone.
Attachments
A screenshot of a page with clipped text, below is the top part of the Activity Monitor window that shows https://kizu.dev process with 100% CPU load. (294.65 KB, image/png)
2024-10-22 14:24 PDT, Roman Komarov
no flags
Radar WebKit Bug Importer
Comment 1 2024-10-22 14:28:57 PDT
Roman Komarov
Comment 2 2024-10-23 02:36:44 PDT
Did some very brief debugging: so far it seems that native nesting implementation could be also at fault. Copying a message from my thread in Mastodon (https://front-end.social/@kizu/113356012201909973): Ok, played a bit with debugging it: and it might be an issue not of `:has()` (or not only), but of a native #CSS nesting implementation in Safari. So, you know how `&` in native nesting is a stand-in for `:is()`? When you look at `:is()`, what do you see? Or, actually, what do you _not_ see? Here, let me make it explicit: *:is() Here, the `*` — a universal selector. Hypothesis: when we use native nesting, Safari will try to match _everything_ against _everything_. What I did to test this hypothesis: in my source code, locally, I replaced this part: https://github.com/kizu/kizu.ru/blob/source/src/posts/2024-10-22-tree-counting-and-random/examples/children-count.html#L26-L27 That currently contains a bunch of native nesting, with an expanded version: .count-and-index-99-children:has(>li:last-child:nth-child(10n+1)) { --cc1: 1 } and so on. This seemed to eliminate the issue. But making it into even a very simple nesting like .count-and-index-99-children { &:has(>li:last-child:nth-child(10n+1)) { --cc1: 1 } } Leads to the issue. In this case, when we do `.foo { &:has(.bar) }`, it can be “expanded” as `*:is(.foo):has(.bar)`, and what happens is Safari does what all browser engines used to do for selectors: tries to match it from right to left! So if first matches `*:has()` — and you can guess where the problem is. I think, I actually managed to work around this by switching where the `&` goes: instead of `.foo { &:has(.bar) }` doing `.foo { :has(.bar)& }` seems to help, which might confirm my hypothesis? The way I feel selectors like these should work: instead of just matching them from right to left, we should first match the parts that should be the fastest, eliminating the more obvious options. `:is()` that does contain a simple selector should be evaluated _before_ `:has()`.
Simon Fraser (smfr)
Comment 3 2024-10-23 09:11:12 PDT
Roman, could you stash a version of the page without the Safari workaround somewhere?
Roman Komarov
Comment 4 2024-10-23 09:23:38 PDT
Sure, here I did create a branch from the point before the fix and deployed it to be able to preview it: https://no-safari-fix--kizu-dev.netlify.app/tree-counting-and-random/ (it freezes for me both on desktop and mobile, both in stable Safari and in TP)
Note You need to log in before you can comment on or make changes to this bug.