From 8f9fe859ad85cdec6c353a6e0d38d797c6333d43 Mon Sep 17 00:00:00 2001 From: David Newell Date: Tue, 9 Apr 2024 10:01:53 +0100 Subject: [PATCH 01/16] better splitting of selectors --- packages/rrweb-snapshot/src/css.ts | 32 +++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 522e257daf..89026efa05 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -429,14 +429,40 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { if (!m) { return; } + + function splitRootSelectors(input: string) { + const parts = []; + let nestedLevel = 0; + let currentPart = ''; + + for (let i = 0; i < input.length; i++) { + const char = input[i]; + currentPart += char; + + if (char === '(') { + nestedLevel++; + } else if (char === ')') { + nestedLevel--; + } else if (char === ',' && nestedLevel === 0) { + parts.push(currentPart.slice(0, -1).trim()); + currentPart = ''; + } + } + + if (currentPart.trim() !== '') { + parts.push(currentPart.trim()); + } + + return parts; + } + /* @fix Remove all comments from selectors * http://ostermiller.org/findcomment.html */ - return trim(m[0]) + return splitRootSelectors(trim(m[0]) .replace(/\/\*([^*]|[\r\n]|(\*+([^*/]|[\r\n])))*\*\/+/g, '') .replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'/g, (m) => { return m.replace(/,/g, '\u200C'); - }) - .split(/\s*(?![^(]*\)),\s*/) + })) .map((s) => { return s.replace(/\u200C/g, ','); }); From 026c6bf8c223dab9e016a29c28d46bd22f0947e2 Mon Sep 17 00:00:00 2001 From: David Newell Date: Tue, 9 Apr 2024 10:08:19 +0100 Subject: [PATCH 02/16] add changeset --- .changeset/modern-doors-watch.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/modern-doors-watch.md diff --git a/.changeset/modern-doors-watch.md b/.changeset/modern-doors-watch.md new file mode 100644 index 0000000000..222a939c46 --- /dev/null +++ b/.changeset/modern-doors-watch.md @@ -0,0 +1,5 @@ +--- +'rrweb-snapshot': patch +--- + +better nested css selector splitting From 729d8ee74a77449d7380a81e5acae60ca3c4ed18 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Tue, 9 Apr 2024 10:33:45 +0100 Subject: [PATCH 03/16] Add test from example at https://github.com/PostHog/posthog/pull/21427 --- packages/rrweb-snapshot/test/css.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 2818386071..1b570644b9 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -119,6 +119,17 @@ describe('css parser', () => { expect(out3).toEqual('[data-aa\\:other] { color: red; }'); }); + it('parses nested commas in selectors correctly', () => { + const result = parse( + ` +body > ul :is(li:not(:first-of-type) a:hover, li:not(:first-of-type).active a) { + background: red; +} +`, + ); + expect((result.stylesheet!.rules[0] as Rule)!.selectors!.length).toEqual(1); + }); + it('parses imports with quotes correctly', () => { const out1 = escapeImportStatement({ cssText: `@import url("/foo.css;900;800"");`, From 84f0f224c0e78c87d144d23933d003814fd3d181 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Tue, 9 Apr 2024 10:50:47 +0100 Subject: [PATCH 04/16] Parsing nesting is hard :( --- packages/rrweb-snapshot/test/css.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 1b570644b9..49b3a2519a 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -128,6 +128,17 @@ body > ul :is(li:not(:first-of-type) a:hover, li:not(:first-of-type).active a) { `, ); expect((result.stylesheet!.rules[0] as Rule)!.selectors!.length).toEqual(1); + + const trickresult = parse( + ` +li[attr="weirdly("] a:hover, li[attr="weirdly)"] a { + background-color: red; +} +`, + ); + expect( + (trickresult.stylesheet!.rules[0] as Rule)!.selectors!.length, + ).toEqual(2); }); it('parses imports with quotes correctly', () => { From 90de2252d5816f1f77f2530104984116e3011d7f Mon Sep 17 00:00:00 2001 From: David Newell Date: Wed, 10 Apr 2024 11:53:54 +0100 Subject: [PATCH 05/16] ignore brackets inside selector strings --- packages/rrweb-snapshot/src/css.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 89026efa05..889591e8b2 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -434,18 +434,25 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { const parts = []; let nestedLevel = 0; let currentPart = ''; + let currentStringChar = null for (let i = 0; i < input.length; i++) { const char = input[i]; currentPart += char; - if (char === '(') { + if (char === '(' && !currentStringChar) { nestedLevel++; - } else if (char === ')') { + } else if (char === ')' && !currentStringChar) { nestedLevel--; } else if (char === ',' && nestedLevel === 0) { parts.push(currentPart.slice(0, -1).trim()); currentPart = ''; + } else if (char.match(/"|'|`/)) { + if (currentStringChar === char) { + currentStringChar = null + } else { + currentStringChar = char + } } } From a4c9643e6c6a252cefebac0a87932e6b733ad7f2 Mon Sep 17 00:00:00 2001 From: David Newell Date: Wed, 10 Apr 2024 11:54:20 +0100 Subject: [PATCH 06/16] run prettier --- packages/rrweb-snapshot/src/css.ts | 53 +++++++++++++++--------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 889591e8b2..6b281ea3fb 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -434,30 +434,30 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { const parts = []; let nestedLevel = 0; let currentPart = ''; - let currentStringChar = null + let currentStringChar = null; for (let i = 0; i < input.length; i++) { - const char = input[i]; - currentPart += char; - - if (char === '(' && !currentStringChar) { - nestedLevel++; - } else if (char === ')' && !currentStringChar) { - nestedLevel--; - } else if (char === ',' && nestedLevel === 0) { - parts.push(currentPart.slice(0, -1).trim()); - currentPart = ''; - } else if (char.match(/"|'|`/)) { - if (currentStringChar === char) { - currentStringChar = null - } else { - currentStringChar = char - } + const char = input[i]; + currentPart += char; + + if (char === '(' && !currentStringChar) { + nestedLevel++; + } else if (char === ')' && !currentStringChar) { + nestedLevel--; + } else if (char === ',' && nestedLevel === 0) { + parts.push(currentPart.slice(0, -1).trim()); + currentPart = ''; + } else if (char.match(/"|'|`/)) { + if (currentStringChar === char) { + currentStringChar = null; + } else { + currentStringChar = char; } + } } if (currentPart.trim() !== '') { - parts.push(currentPart.trim()); + parts.push(currentPart.trim()); } return parts; @@ -465,14 +465,15 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { /* @fix Remove all comments from selectors * http://ostermiller.org/findcomment.html */ - return splitRootSelectors(trim(m[0]) - .replace(/\/\*([^*]|[\r\n]|(\*+([^*/]|[\r\n])))*\*\/+/g, '') - .replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'/g, (m) => { - return m.replace(/,/g, '\u200C'); - })) - .map((s) => { - return s.replace(/\u200C/g, ','); - }); + return splitRootSelectors( + trim(m[0]) + .replace(/\/\*([^*]|[\r\n]|(\*+([^*/]|[\r\n])))*\*\/+/g, '') + .replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'/g, (m) => { + return m.replace(/,/g, '\u200C'); + }), + ).map((s) => { + return s.replace(/\u200C/g, ','); + }); } /** From ca41041939a6f8766305d5d329fac479faffb860 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 10 Apr 2024 14:51:16 +0100 Subject: [PATCH 07/16] Add another test as noticed that it's possible to escape strings --- packages/rrweb-snapshot/test/css.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 49b3a2519a..85506fd58c 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -139,6 +139,17 @@ li[attr="weirdly("] a:hover, li[attr="weirdly)"] a { expect( (trickresult.stylesheet!.rules[0] as Rule)!.selectors!.length, ).toEqual(2); + + const weirderresult = parse( + ` +li[attr="weirder\"("] a:hover, li[attr="weirder\")"] a { + background-color: red; +} +`, + ); + expect( + (weirderresult.stylesheet!.rules[0] as Rule)!.selectors!.length, + ).toEqual(2); }); it('parses imports with quotes correctly', () => { From f61224763b5a8ae6ba4f0edde0912feeb904464f Mon Sep 17 00:00:00 2001 From: David Newell Date: Wed, 10 Apr 2024 15:18:27 +0100 Subject: [PATCH 08/16] Update packages/rrweb-snapshot/src/css.ts Co-authored-by: Eoghan Murray --- packages/rrweb-snapshot/src/css.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 6b281ea3fb..9e9b6a3a3d 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -447,7 +447,7 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { } else if (char === ',' && nestedLevel === 0) { parts.push(currentPart.slice(0, -1).trim()); currentPart = ''; - } else if (char.match(/"|'|`/)) { + } else if ("'\"".includes(char)) { if (currentStringChar === char) { currentStringChar = null; } else { From c9089ffbfc114a515ff8b481b182c1d288502c86 Mon Sep 17 00:00:00 2001 From: David Newell Date: Wed, 10 Apr 2024 18:38:59 +0100 Subject: [PATCH 09/16] appease the prettier --- packages/rrweb-snapshot/src/css.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 9e9b6a3a3d..62611b6c0b 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -447,7 +447,7 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { } else if (char === ',' && nestedLevel === 0) { parts.push(currentPart.slice(0, -1).trim()); currentPart = ''; - } else if ("'\"".includes(char)) { + } else if ('\'"'.includes(char)) { if (currentStringChar === char) { currentStringChar = null; } else { From eaaf612b1f0f58df19ecbf6d5242b6c6c135a3dc Mon Sep 17 00:00:00 2001 From: David Newell Date: Fri, 12 Apr 2024 12:32:51 +0100 Subject: [PATCH 10/16] Update packages/rrweb-snapshot/src/css.ts Co-authored-by: Eoghan Murray --- packages/rrweb-snapshot/src/css.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 62611b6c0b..7add1bf914 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -449,7 +449,9 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { currentPart = ''; } else if ('\'"'.includes(char)) { if (currentStringChar === char) { - currentStringChar = null; + if (!(i > 0 && input[i - 1] === '\\')) { + currentStringChar = null; + } } else { currentStringChar = char; } From 80c8fb1999c26cbf9c0c8ff0330a595fdfc50885 Mon Sep 17 00:00:00 2001 From: David Newell Date: Fri, 12 Apr 2024 17:15:25 +0100 Subject: [PATCH 11/16] Update packages/rrweb-snapshot/test/css.test.ts --- packages/rrweb-snapshot/test/css.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 85506fd58c..49b3a2519a 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -139,17 +139,6 @@ li[attr="weirdly("] a:hover, li[attr="weirdly)"] a { expect( (trickresult.stylesheet!.rules[0] as Rule)!.selectors!.length, ).toEqual(2); - - const weirderresult = parse( - ` -li[attr="weirder\"("] a:hover, li[attr="weirder\")"] a { - background-color: red; -} -`, - ); - expect( - (weirderresult.stylesheet!.rules[0] as Rule)!.selectors!.length, - ).toEqual(2); }); it('parses imports with quotes correctly', () => { From 4234fa01995800725bbb3350190300b01253e242 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Thu, 18 Apr 2024 16:12:23 +0100 Subject: [PATCH 12/16] Revert "Update packages/rrweb-snapshot/test/css.test.ts" This reverts commit 80c8fb1999c26cbf9c0c8ff0330a595fdfc50885. --- packages/rrweb-snapshot/test/css.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 49b3a2519a..85506fd58c 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -139,6 +139,17 @@ li[attr="weirdly("] a:hover, li[attr="weirdly)"] a { expect( (trickresult.stylesheet!.rules[0] as Rule)!.selectors!.length, ).toEqual(2); + + const weirderresult = parse( + ` +li[attr="weirder\"("] a:hover, li[attr="weirder\")"] a { + background-color: red; +} +`, + ); + expect( + (weirderresult.stylesheet!.rules[0] as Rule)!.selectors!.length, + ).toEqual(2); }); it('parses imports with quotes correctly', () => { From 89fd41fb3c404779cd012ce38716f5271ef93fb6 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Thu, 18 Apr 2024 16:19:55 +0100 Subject: [PATCH 13/16] Reinstate 'Add another test as noticed that it's possible to escape strings' - I needed to escape the backslash to demonstrate the test case --- packages/rrweb-snapshot/test/css.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 85506fd58c..446eb29cf7 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -142,7 +142,7 @@ li[attr="weirdly("] a:hover, li[attr="weirdly)"] a { const weirderresult = parse( ` -li[attr="weirder\"("] a:hover, li[attr="weirder\")"] a { +li[attr="weirder\\"("] a:hover, li[attr="weirder\\")"] a { background-color: red; } `, From 4091fbd24d6c294b9117d3f59cd3110db3261139 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Thu, 18 Apr 2024 16:25:21 +0100 Subject: [PATCH 14/16] Ensure we are ignoring commas within strings - the added test case didn't actually fail before due to some voodoo with `m.replace(/,/g, '\u200C');` which took me ages to figure out. Anyhow I figure test and refactor is good in case we replace the css library --- packages/rrweb-snapshot/src/css.ts | 18 +++++++++--------- packages/rrweb-snapshot/test/css.test.ts | 11 +++++++++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 7add1bf914..e65a67cf8c 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -440,21 +440,21 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { const char = input[i]; currentPart += char; - if (char === '(' && !currentStringChar) { + const hasStringEscape = i > 0 && input[i - 1] === '\\'; + + if (currentStringChar) { + if (currentStringChar === char && !hasStringEscape) { + currentStringChar = null; + } + } else if (char === '(') { nestedLevel++; - } else if (char === ')' && !currentStringChar) { + } else if (char === ')') { nestedLevel--; } else if (char === ',' && nestedLevel === 0) { parts.push(currentPart.slice(0, -1).trim()); currentPart = ''; } else if ('\'"'.includes(char)) { - if (currentStringChar === char) { - if (!(i > 0 && input[i - 1] === '\\')) { - currentStringChar = null; - } - } else { - currentStringChar = char; - } + currentStringChar = char; } } diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 446eb29cf7..3f131e976b 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -150,6 +150,17 @@ li[attr="weirder\\"("] a:hover, li[attr="weirder\\")"] a { expect( (weirderresult.stylesheet!.rules[0] as Rule)!.selectors!.length, ).toEqual(2); + + const commainstrresult = parse( + ` +li[attr="has,comma"] a:hover { + background-color: red; +} +`, + ); + expect( + (commainstrresult.stylesheet!.rules[0] as Rule)!.selectors!.length, + ).toEqual(1); }); it('parses imports with quotes correctly', () => { From 7646ce10fd6c5e12dbfc053d7b09394a6c896a4b Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Thu, 18 Apr 2024 23:22:20 +0100 Subject: [PATCH 15/16] Update .changeset/modern-doors-watch.md update changeset to reflect more limited effect --- .changeset/modern-doors-watch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/modern-doors-watch.md b/.changeset/modern-doors-watch.md index 222a939c46..b74bfbad45 100644 --- a/.changeset/modern-doors-watch.md +++ b/.changeset/modern-doors-watch.md @@ -2,4 +2,4 @@ 'rrweb-snapshot': patch --- -better nested css selector splitting +better nested css selector splitting when commas or brackets happen to be in quoted text From fb91382f253ef4dec31184f10533c1936ec7f64b Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Thu, 18 Apr 2024 23:25:55 +0100 Subject: [PATCH 16/16] Prettier --- packages/rrweb-snapshot/src/css.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index df62b965a1..1a3157d40f 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -467,7 +467,6 @@ export function parse(css: string, options: ParserOptions = {}): Stylesheet { let currentStringChar = null; for (const char of input) { - const hasStringEscape = currentSegment.endsWith('\\'); if (currentStringChar) {