Revert "fix: Incorrect parsing of functional pseudo class css selector (

#169)" (#182) This reverts commit 810b39f. Reverting this in favor of rrweb-io#1401 and rrweb-io#1440 which is better than whatever it is I wrote. I kept our tests just to ensure the fix is compatible with our previous patch.
getsentry · Apr 19, 2024 · de6cd2b · de6cd2b
2 parents 9e07245 + 4c76050
commit de6cd2b
Show file tree

Hide file tree

Showing 4 changed files with 147 additions and 87 deletions.
diff --git a/.changeset/modern-doors-watch.md b/.changeset/modern-doors-watch.md
@@ -0,0 +1,5 @@
+---
+'rrweb-snapshot': patch
+---
+
+better nested css selector splitting when commas or brackets happen to be in quoted text
diff --git a/.changeset/rich-dots-lay.md b/.changeset/rich-dots-lay.md
@@ -0,0 +1,5 @@
+---
+'rrweb-snapshot': patch
+---
+
+Fix css parsing errors
diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts
@@ -431,102 +431,81 @@ export function parse(css: string, options: ParserOptions = {}) {
    */
 
   function selector() {
-    const m = match(/^([^{]+)/);
+    whitespace();
+    while (css[0] == '}') {
+      error('extra closing bracket');
+      css = css.slice(1);
+      whitespace();
+    }
+
+    // Use match logic from https://github.com/NxtChg/pieces/blob/3eb39c8287a97632e9347a24f333d52d916bc816/js/css_parser/css_parse.js#L46C1-L47C1
+    const m = match(/^(("(?:\\"|[^"])*"|'(?:\\'|[^'])*'|[^{])+)/);
     if (!m) {
       return;
     }
+
     /* @fix Remove all comments from selectors
      * http://ostermiller.org/findcomment.html */
-    const splitSelectors = trim(m[0])
+    const cleanedInput = m[0]
+      .trim()
       .replace(/\/\*([^*]|[\r\n]|(\*+([^*/]|[\r\n])))*\*\/+/g, '')
+
+      // Handle strings by replacing commas inside them
       .replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'/g, (m) => {
         return m.replace(/,/g, '\u200C');
-      })
-      .split(/\s*(?![^(]*\)),\s*/);
-
-    if (splitSelectors.length <= 1) {
-      return splitSelectors.map((s) => {
-        return s.replace(/\u200C/g, ',');
       });
-    }
 
-    // For each selector, need to check if we properly split on `,`
-    // Example case where selector is:
-    // .bar:has(input:is(:disabled), button:is(:disabled))
-    let i = 0;
-    let j = 0;
-    const len = splitSelectors.length;
-    const finalSelectors = [];
-    while (i < len) {
-      // Look for selectors with opening parens - `(` and search rest of
-      // selectors for the first one with matching number of closing
-      // parens `)`
-      const openingParensCount = (splitSelectors[i].match(/\(/g) || []).length;
-      const closingParensCount = (splitSelectors[i].match(/\)/g) || []).length;
-      let unbalancedParens = openingParensCount - closingParensCount;
-
-      if (unbalancedParens >= 1) {
-        // At least one opening parens was found, prepare to look through
-        // rest of selectors
-        let foundClosingSelector = false;
-
-        // Loop starting with next item in array, until we find matching
-        // number of ending parens
-        j = i + 1;
-        while (j < len) {
-          // peek into next item to count the number of closing brackets
-          const nextOpeningParensCount = (splitSelectors[j].match(/\(/g) || [])
-            .length;
-          const nextClosingParensCount = (splitSelectors[j].match(/\)/g) || [])
-            .length;
-          const nextUnbalancedParens =
-            nextClosingParensCount - nextOpeningParensCount;
-
-          if (nextUnbalancedParens === unbalancedParens) {
-            // Matching # of closing parens was found, join all elements
-            // from i to j
-            finalSelectors.push(splitSelectors.slice(i, j + 1).join(','));
-
-            // we will want to skip the items that we have joined together
-            i = j + 1;
-
-            // Use to continue the outer loop
-            foundClosingSelector = true;
-
-            // break out of inner loop so we found matching closing parens
-            break;
-          }
-
-          // No matching closing parens found, keep moving through index, but
-          // update the # of unbalanced parents still outstanding
-          j++;
-          unbalancedParens -= nextUnbalancedParens;
-        }
+    // Split using a custom function and restore commas in strings
+    return customSplit(cleanedInput).map((s) =>
+      s.replace(/\u200C/g, ',').trim(),
+    );
+  }
+
+  /**
+   * Split selector correctly, ensuring not to split on comma if inside ().
+   */
+
+  function customSplit(input: string) {
+    const result = [];
+    let currentSegment = '';
+    let depthParentheses = 0; // Track depth of parentheses
+    let depthBrackets = 0; // Track depth of square brackets
+    let currentStringChar = null;
 
-        if (foundClosingSelector) {
-          // Matching closing selector was found, move to next selector
-          continue;
+    for (const char of input) {
+      const hasStringEscape = currentSegment.endsWith('\\');
+
+      if (currentStringChar) {
+        if (currentStringChar === char && !hasStringEscape) {
+          currentStringChar = null;
         }
+      } else if (char === '(') {
+        depthParentheses++;
+      } else if (char === ')') {
+        depthParentheses--;
+      } else if (char === '[') {
+        depthBrackets++;
+      } else if (char === ']') {
+        depthBrackets--;
+      } else if ('\'"'.includes(char)) {
+        currentStringChar = char;
+      }
 
-        // No matching closing selector was found, either invalid CSS,
-        // or unbalanced number of opening parens were used as CSS
-        // selectors. Assume that rest of the list of selectors are
-        // selectors and break to avoid iterating through the list of
-        // selectors again.
-        splitSelectors
-          .slice(i, len)
-          .forEach((selector) => selector && finalSelectors.push(selector));
-        break;
+      // Split point is a comma that is not inside parentheses or square brackets
+      if (char === ',' && depthParentheses === 0 && depthBrackets === 0) {
+        result.push(currentSegment);
+        currentSegment = '';
+      } else {
+        currentSegment += char;
       }
+    }
 
-      // No opening parens found, contiue looking through list
-      splitSelectors[i] && finalSelectors.push(splitSelectors[i]);
-      i++;
+    // Add the last segment
+    if (currentSegment) {
+      result.push(currentSegment);
     }
 
-    return finalSelectors.map((s) => {
-      return s.replace(/\u200C/g, ',');
-    });
+    return result;
   }
 
   /**

diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts
@@ -78,6 +78,35 @@ describe('css parser', () => {
     expect(errors[0].filename).toEqual('foo.css');
   });
 
+  it('should parse selector with comma nested inside ()', () => {
+    const result = parse(
+      '[_nghost-ng-c4172599085]:not(.fit-content).aim-select:hover:not(:disabled, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--disabled, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--invalid, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--active) { border-color: rgb(84, 84, 84); }',
+    );
+
+    expect(result.parent).toEqual(null);
+
+    const rules = result.stylesheet!.rules;
+    expect(rules.length).toEqual(1);
+
+    let rule = rules[0] as Rule;
+    expect(rule.parent).toEqual(result);
+    expect(rule.selectors?.length).toEqual(1);
+
+    let decl = rule.declarations![0];
+    expect(decl.parent).toEqual(rule);
+  });
+
+  it('parses { and } in attribute selectors correctly', () => {
+    const result = parse('foo[someAttr~="{someId}"] { color: red; }');
+    const rules = result.stylesheet!.rules;
+
+    expect(rules.length).toEqual(1);
+
+    const rule = rules[0] as Rule;
+
+    expect(rule.selectors![0]).toEqual('foo[someAttr~="{someId}"]');
+  });
+
   it('should set parent property', () => {
     const result = parse(
       'thing { test: value; }\n' +
@@ -119,6 +148,50 @@ 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);
+
+    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);
+
+    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);
+
+    const commainstrresult = parse(
+      `
+li[attr="has,comma"] a:hover {
+  background-color: red;
+}
+`,
+    );
+    expect(
+      (commainstrresult.stylesheet!.rules[0] as Rule)!.selectors!.length,
+    ).toEqual(1);
+  });
+
   it.each([
     ['.foo,.bar {}', ['.foo', '.bar']],
     ['.bar:has(:disabled) {}', ['.bar:has(:disabled)']],
@@ -129,11 +202,11 @@ describe('css parser', () => {
     ],
     [
       '.bar:has(div, input:is(:disabled), button) {}',
-      ['.bar:has(div,input:is(:disabled), button)'],
+      ['.bar:has(div, input:is(:disabled), button)'],
     ],
     [
       '.bar:has(div, input:is(:disabled),button:has(:disabled,.baz)) {}',
-      ['.bar:has(div,input:is(:disabled),button:has(:disabled,.baz))'],
+      ['.bar:has(div, input:is(:disabled),button:has(:disabled,.baz))'],
     ],
     [
       '.bar:has(input), .foo:has(input, button), .baz {}',
@@ -142,17 +215,15 @@ describe('css parser', () => {
     [
       '.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz)){color: red;}',
       [
-        '.bar:has(input:is(:disabled),button:has(:disabled,.baz),div:has(:disabled,.baz))',
+        '.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz))',
       ],
     ],
-    ['.bar((( {}', ['.bar(((']],
     [
       '.bar:has(:has(:has(a), :has(:has(:has(b, :has(a), c), e))), input:is(:disabled), button) {}',
       [
-        '.bar:has(:has(:has(a),:has(:has(:has(b,:has(a), c), e))),input:is(:disabled), button)',
+        '.bar:has(:has(:has(a), :has(:has(:has(b, :has(a), c), e))), input:is(:disabled), button)',
       ],
     ],
-    ['.foo,.bar(((,.baz {}', ['.foo', '.bar(((', '.baz']],
     [
       '.foo,.bar:has(input:is(:disabled)){color: red;}',
       ['.foo', '.bar:has(input:is(:disabled))'],
@@ -165,14 +236,14 @@ describe('css parser', () => {
       '.foo,.bar:has(input:is(:disabled),button:has(:disabled), div:has(:disabled,.baz)){color: red;}',
       [
         '.foo',
-        '.bar:has(input:is(:disabled),button:has(:disabled),div:has(:disabled,.baz))',
+        '.bar:has(input:is(:disabled),button:has(:disabled), div:has(:disabled,.baz))',
       ],
     ],
     [
       '.foo,.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz)){color: red;}',
       [
         '.foo',
-        '.bar:has(input:is(:disabled),button:has(:disabled,.baz),div:has(:disabled,.baz))',
+        '.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz))',
       ],
     ],
     ['.bar:has(:disabled), .foo {}', ['.bar:has(:disabled)', '.foo']],