[Bug]: addHoverClass creating invalid css

[dengelke]

Preflight Checklist

• I have searched the issue tracker for a bug report that matches the one I want to file, without success.

What package is this bug report for?

rrweb-snapshot

Version

2.0.0-alpha.11

Expected Behavior

Styling should work

Actual Behavior

Replay is crashing due to stylesheet becoming invalid on replay from the addHoverClass function

Steps to Reproduce

Is you have a selector like

[_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); }

then it will be replaced in the addHoverClass by

[_nghost-ng-c4172599085]:not(.fit-content).aim-select:hover:not(:disabled, [_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);
    }

Which is invalid css.

It looks to be something to do with the logic in the parsing, where the selectors looks like

[_nghost-ng-c4172599085]:not(.fit-content).aim-select:hover:not(:disabled

Which is then getting fed into the addHoverClass function

Testcase Gist URL

No response

Additional Information

No response

[jaj1014]

I'm having a similar issue to the one described above. Here is some more detail that I was able to gather.

The issue I hit is with how parse() handles selectors which have curly braces in them. These can show up when the selector is an attribute selector. Ex. [data-special-attr~="{moreSpecialCode}"].inner-element The regex used to parse selectors basically splits on {. In the case of the valid selector presented above, that means it would split it up into [data-special-attr~= as the selector. The selector the AST now contains is invalid and can lead to issues when adding the 'hover' class. This will cause the replay to be styled incorrectly.

While these selectors are not ideal, the app I found the issue with was using this framework that appends an html attribute (data-ractive-css="{randomUUID}") to elements of a component when rendered to the DOM to aid in scoping styles. The curly braces are always present, and the css selectors end up looking something like the example from the above paragraph. I'm sure there are other instances, too.

I did try to implement a fix locally and was successful for the recording in which I discovered the issue. I created a jest test case (below) and was able to get it to pass, but the regex update I tried (inside selector() ->/([^{}]*\{[^{}]*\})?([^{};]+)[^{};]*/) caused several others of the tests css.test.ts to fail. So I don't have high confidence it's the right approach/solution (it was also a ChatGPT special since I'm terrible at Regex)

Test case I would expect to pass:

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}"]');
});

In searching the rrweb issues this, I found evidence which pointed me to parse() code being forked from reworkcss. I noticed that library has bugs open for similar things re: parsing errors. Even their current regex implementation for selector() naively uses the same /^([^{]+)/ which doesn't account for { existing in the selector. I was hoping for a quick win that maybe they had already updated their code and it could just be ported over.

Lastly, I was also able to create a demo recording (link to gist and recording) to see what this looks like in a player. And for comparison, here is a gif of what the test page should actually do.

[dengelke]

Ok I think I've found a fix for both my issue and the issue raised by @jaj1014 using https://github.com/NxtChg/pieces/blob/master/js/css_parser/css_parse.js as a basis

[dengelke]

@jaj1014 see #1401 for a fix for your problem that also resolves mine!