Fix splitCssText again (#1640)
Fixes a browser 'lock up' at record time due to a presence of large amounts of css in <style> elements, which are split over multiple text nodes, which triggers the new code added in #1437 (see that PR for full explanation of why this all exists). #1437 was not written with performance in mind as it was believed to be an edge case, but things like Grammarly browser extension (#1603) among other scenarios were triggering pathological behavior, some of which was solved in #1615. See also https://github.com/rrweb-io/rrweb/pull/1640#issuecomment-2633505804 for further discussion. * Fix the case when there are multiple matches and we end up not finding a unique one - just go with the best guess when there are many splits by looking at the previous chunk's size * Also add '0px' -> '0' stylesheet normalization, which also fixes the sample problem in a different way * Add new test and modify it so that it can trigger a failure in the absence of the '0px' normalization; there may be other unknown ways of triggering a similar bug, so ensure that the primary 'best guess' method doesn't suffer a regression * Leverage the 'best guess' method so that we can quit after 100 iterations trying to find a unique substring; hopefully this bit along with the `iterLimit` already added will prevent any future pathological cases. Failing example extracted from large files identified by Paul D'Ambra (Posthog) ... see comment from MartinWorkfully: https://github.com/PostHog/posthog-js/issues/1668
This commit is contained in:
@@ -178,7 +178,6 @@ describe('css splitter', () => {
|
||||
transition: all 4s ease;
|
||||
}`),
|
||||
);
|
||||
// TODO: splitCssText can't handle it yet if both start with .x
|
||||
style.appendChild(
|
||||
JSDOM.fragment(`.y {
|
||||
-moz-transition: all 5s ease;
|
||||
@@ -227,6 +226,89 @@ describe('css splitter', () => {
|
||||
}
|
||||
expect(splitCssText(cssText, style)).toEqual(sections);
|
||||
});
|
||||
|
||||
it('finds css textElement splits correctly, with substring matching going from many to none', () => {
|
||||
const window = new Window({ url: 'https://localhost:8080' });
|
||||
const document = window.document;
|
||||
document.head.innerHTML = `<style>
|
||||
.section-news-v3-detail .news-cnt-wrapper :where(p):not(:where([class~="not-prose"], [class~="not-prose"] *)) {
|
||||
margin-top: 0px;
|
||||
margin-bottom: 0px;
|
||||
}
|
||||
|
||||
.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 :where(figure):not(:where([class~="not-prose"],[class~="not-prose"] *)) {
|
||||
margin-top: 2em;
|
||||
margin-bottom: 2em;
|
||||
}
|
||||
|
||||
.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 :where(.prose > :first-child):not(:where([class~="not-prose"],[cl</style>`;
|
||||
const style = document.querySelector('style');
|
||||
if (style) {
|
||||
// happydom? bug avoid: strangely a greater than symbol in the template string below
|
||||
// e.g. '.prose > :last-child' causes more than one child to be appended
|
||||
style.append(`ass~="not-prose"] *)) {
|
||||
margin-top: 0; /* cssRules transforms this to '0px' which was preventing matching prior to normalization */
|
||||
}
|
||||
|
||||
.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 :where(.prose :last-child):not(:where([class~="not-prose"],[class~="not-prose"] *)) {
|
||||
margin-bottom: 0;
|
||||
}
|
||||
|
||||
.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 {
|
||||
width: 100%;
|
||||
overflow-wrap: break-word;
|
||||
}
|
||||
|
||||
.section-home {
|
||||
height: 100%;
|
||||
overflow-y: auto;
|
||||
}
|
||||
`);
|
||||
|
||||
expect(style.childNodes.length).toEqual(2);
|
||||
|
||||
const expected = [
|
||||
'.section-news-v3-detail .news-cnt-wrapper :where(p):not(:where([class~="not-prose"], [class~="not-prose"] *)) { margin-top: 0px; margin-bottom: 0px; }.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 :where(figure):not(:where([class~="not-prose"],[class~="not-prose"] *)) { margin-top: 2em; margin-bottom: 2em; }.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 :where(.prose > :first-child):not(:where([class~="not-prose"],[cl',
|
||||
'ass~="not-prose"] *)) { margin-top: 0px; }.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 :where(.prose :last-child):not(:where([class~="not-prose"],[class~="not-prose"] *)) { margin-bottom: 0px; }.section-news-v3-detail .news-cnt-wrapper .plugins-wrapper2 { width: 100%; overflow-wrap: break-word; }.section-home { height: 100%; overflow-y: auto; }',
|
||||
];
|
||||
const browserSheet = expected.join('');
|
||||
expect(stringifyStylesheet(style.sheet!)).toEqual(browserSheet);
|
||||
let _testNoPxNorm = true; // trigger the original motivating scenario for this test
|
||||
expect(splitCssText(browserSheet, style, _testNoPxNorm)).toEqual(
|
||||
expected,
|
||||
);
|
||||
_testNoPxNorm = false; // this case should also be solved by normalizing '0px' -> '0'
|
||||
expect(splitCssText(browserSheet, style, _testNoPxNorm)).toEqual(
|
||||
expected,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('finds css textElement splits correctly, even with repeated sections', () => {
|
||||
const window = new Window({ url: 'https://localhost:8080' });
|
||||
const document = window.document;
|
||||
document.head.innerHTML =
|
||||
'<style>.a{background-color: black; } </style>';
|
||||
const style = document.querySelector('style');
|
||||
if (style) {
|
||||
style.append('.x{background-color:red;}');
|
||||
style.append('.b {background-color:black;}');
|
||||
style.append('.x{background-color:red;}');
|
||||
style.append('.c{ background-color: black}');
|
||||
|
||||
const expected = [
|
||||
'.a { background-color: black; }',
|
||||
'.x { background-color: red; }',
|
||||
'.b { background-color: black; }',
|
||||
'.x { background-color: red; }',
|
||||
'.c { background-color: black; }',
|
||||
];
|
||||
const browserSheet = expected.join('');
|
||||
expect(stringifyStylesheet(style.sheet!)).toEqual(browserSheet);
|
||||
|
||||
expect(splitCssText(browserSheet, style)).toEqual(expected);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('applyCssSplits css rejoiner', function () {
|
||||
|
||||
Reference in New Issue
Block a user