Fix lit based diff to match legacy diff exactly (Step 1) The lit diff contains a <gr-diff-text> component wrapper in each content cell. Let's also use a (no-op) wrapper (and give it a similar name: <gr-legacy-text>) to the legacy diff for easier comparison of the DOM output of the two. And also for making the two diffs behave truly identical with respect to query selectors, etc. This also allows us to easily ignore the actual text of the diff in gr-diff_test, which is nice, because that test is only about the structure of the diff and not about the content of each line. Release-Notes: skip Google-Bug-Id: b/237393560 Change-Id: I579d6f5c452d9d106b5bce41d7c0b9ca07296752
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts index 210b8b2..5eabc59 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
@@ -71,7 +71,7 @@ const text = 'a'.repeat(51); const expected = 'a'.repeat(50) + WBR_HTML + 'a'; const result = builder.createTextEl(null, line(text)).firstElementChild - ?.innerHTML; + ?.firstElementChild?.innerHTML; assert.equal(result, expected); }); @@ -80,7 +80,7 @@ const text = 'a'.repeat(51); const expected = 'a'.repeat(50) + LINE_BREAK_HTML + 'a'; const result = builder.createTextEl(null, line(text)).firstElementChild - ?.innerHTML; + ?.firstElementChild?.innerHTML; assert.equal(result, expected); }); @@ -119,7 +119,7 @@ assert.equal(el.innerText, text); // With line length 10 and tab size 4, there should be a line break // after every two tabs. - const newlineEl = el.querySelector('.contentText > .br'); + const newlineEl = el.querySelector('.contentText .br'); assert.isOk(newlineEl); assert.equal( el.querySelector('.contentText .tab:nth-child(2)')?.nextSibling,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts index 8a17611..f583c2e 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts
@@ -274,6 +274,12 @@ elementId: string ): HTMLElement { const contentText = createElementDiff('div', 'contentText'); + // <gr-legacy-text> is not defined anywhere, so this behave just as a <div> + // would. We use this during the migration to lit based diff elements to + // match <gr-diff-text>. We define a css rule with `display:contents` making + // sure that this extra element is basically a no-op. + const legacyText = document.createElement('gr-legacy-text'); + contentText.appendChild(legacyText); contentText.id = elementId; let columnPos = 0; let textOffset = 0; @@ -285,16 +291,16 @@ let rowStart = 0; let rowEnd = lineLimit - columnPos; while (rowEnd < segment.length) { - contentText.appendChild( + legacyText.appendChild( document.createTextNode(segment.substring(rowStart, rowEnd)) ); - contentText.appendChild(createLineBreak(responsiveMode)); + legacyText.appendChild(createLineBreak(responsiveMode)); columnPos = 0; rowStart = rowEnd; rowEnd += lineLimit; } // Append the last part of |segment|, which fits on the current line. - contentText.appendChild( + legacyText.appendChild( document.createTextNode(segment.substring(rowStart)) ); columnPos += segment.length - rowStart; @@ -306,20 +312,20 @@ // Append a single '\t' character. let effectiveTabSize = tabSize - (columnPos % tabSize); if (columnPos + effectiveTabSize > lineLimit) { - contentText.appendChild(createLineBreak(responsiveMode)); + legacyText.appendChild(createLineBreak(responsiveMode)); columnPos = 0; effectiveTabSize = tabSize; } - contentText.appendChild(createTabWrapper(effectiveTabSize)); + legacyText.appendChild(createTabWrapper(effectiveTabSize)); columnPos += effectiveTabSize; textOffset++; } else { // Append a single surrogate pair. if (columnPos >= lineLimit) { - contentText.appendChild(createLineBreak(responsiveMode)); + legacyText.appendChild(createLineBreak(responsiveMode)); columnPos = 0; } - contentText.appendChild( + legacyText.appendChild( document.createTextNode(text.substring(textOffset, textOffset + 2)) ); textOffset += 2;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts index 7e8eb4c..98b4586 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts
@@ -20,10 +20,13 @@ test('formatText newlines 1', () => { let text = 'abcdef'; - assert.equal(formatText(text, 'NONE', 4, 10, '').innerHTML, text); + assert.equal( + formatText(text, 'NONE', 4, 10, '').firstElementChild?.innerHTML, + text + ); text = 'a'.repeat(20); assert.equal( - formatText(text, 'NONE', 4, 10, '').innerHTML, + formatText(text, 'NONE', 4, 10, '').firstElementChild?.innerHTML, 'a'.repeat(10) + LINE_BREAK_HTML + 'a'.repeat(10) ); }); @@ -31,7 +34,7 @@ test('formatText newlines 2', () => { const text = '<span class="thumbsup">👍</span>'; assert.equal( - formatText(text, 'NONE', 4, 10, '').innerHTML, + formatText(text, 'NONE', 4, 10, '').firstElementChild?.innerHTML, '<span clas' + LINE_BREAK_HTML + 's="thumbsu' + @@ -45,7 +48,7 @@ test('formatText newlines 3', () => { const text = '01234\t56789'; assert.equal( - formatText(text, 'NONE', 4, 10, '').innerHTML, + formatText(text, 'NONE', 4, 10, '').firstElementChild?.innerHTML, '01234' + createTabWrapper(3).outerHTML + '56' + LINE_BREAK_HTML + '789' ); }); @@ -53,7 +56,7 @@ test('formatText newlines 4', () => { const text = '👍'.repeat(58); assert.equal( - formatText(text, 'NONE', 4, 20, '').innerHTML, + formatText(text, 'NONE', 4, 20, '').firstElementChild?.innerHTML, '👍'.repeat(20) + LINE_BREAK_HTML + '👍'.repeat(20) + @@ -82,7 +85,8 @@ assert.ok(wrapper); assert.equal(wrapper.innerText, '\t'); assert.equal( - formatText(html, 'NONE', tabSize, Infinity, '').innerHTML, + formatText(html, 'NONE', tabSize, Infinity, '').firstElementChild + ?.innerHTML, 'abc' + wrapper.outerHTML + 'def' ); }); @@ -91,31 +95,22 @@ let input = '<script>alert("XSS");<' + '/script>'; let expected = '<script>alert("XSS");</script>'; - let result = formatText( - input, - 'NONE', - 1, - Number.POSITIVE_INFINITY, - '' - ).innerHTML; + let result = formatText(input, 'NONE', 1, Number.POSITIVE_INFINITY, '') + .firstElementChild?.innerHTML; assert.equal(result, expected); input = '& < > " \' / `'; expected = '& < > " \' / `'; - result = formatText( - input, - 'NONE', - 1, - Number.POSITIVE_INFINITY, - '' - ).innerHTML; + result = formatText(input, 'NONE', 1, Number.POSITIVE_INFINITY, '') + .firstElementChild?.innerHTML; assert.equal(result, expected); }); test('text length with tabs and unicode', () => { function expectTextLength(text: string, tabSize: number, expected: number) { // Formatting to |expected| columns should not introduce line breaks. - const result = formatText(text, 'NONE', tabSize, expected, ''); + const result = formatText(text, 'NONE', tabSize, expected, '') + .firstElementChild!; assert.isNotOk( result.querySelector('.contentText > .br'), ' Expected the result of: \n' + @@ -126,19 +121,22 @@ // Increasing the line limit should produce the same markup. assert.equal( - formatText(text, 'NONE', tabSize, Infinity, '').innerHTML, + formatText(text, 'NONE', tabSize, Infinity, '').firstElementChild + ?.innerHTML, result.innerHTML ); assert.equal( - formatText(text, 'NONE', tabSize, expected + 1, '').innerHTML, + formatText(text, 'NONE', tabSize, expected + 1, '').firstElementChild + ?.innerHTML, result.innerHTML ); // Decreasing the line limit should introduce line breaks. if (expected > 0) { - const tooSmall = formatText(text, 'NONE', tabSize, expected - 1, ''); + const tooSmall = formatText(text, 'NONE', tabSize, expected - 1, '') + .firstElementChild!; assert.isOk( - tooSmall.querySelector('.contentText > .br'), + tooSmall.querySelector('.contentText .br'), ' Expected the result of: \n' + ` _formatText(${text}', ${tabSize}, ${expected - 1})\n` + ' to contain a br. But the actual result HTML was:\n' +
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts index 00e46c7..fa15dcf 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -184,9 +184,7 @@ class="contentText gr-diff" data-side="left" id="left-content-1" - > - Lorem ipsum dolor sit amet, suspendisse inceptos vehicula. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="1"> <button @@ -205,9 +203,7 @@ class="contentText gr-diff" data-side="right" id="right-content-1" - > - Lorem ipsum dolor sit amet, suspendisse inceptos vehicula. - </div> + ></div> </td> </tr> <tr @@ -235,9 +231,7 @@ class="contentText gr-diff" data-side="left" id="left-content-2" - > - Mattis lectus. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="2"> <button @@ -256,9 +250,7 @@ class="contentText gr-diff" data-side="right" id="right-content-2" - > - Mattis lectus. - </div> + ></div> </td> </tr> <tr @@ -286,9 +278,7 @@ class="contentText gr-diff" data-side="left" id="left-content-3" - > - Sodales duis. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="3"> <button @@ -307,9 +297,7 @@ class="contentText gr-diff" data-side="right" id="right-content-3" - > - Sodales duis. - </div> + ></div> </td> </tr> <tr @@ -337,9 +325,7 @@ class="contentText gr-diff" data-side="left" id="left-content-4" - > - Orci a faucibus. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="4"> <button @@ -358,9 +344,7 @@ class="contentText gr-diff" data-side="right" id="right-content-4" - > - Orci a faucibus. - </div> + ></div> </td> </tr> </tbody> @@ -399,9 +383,7 @@ class="contentText gr-diff" data-side="right" id="right-content-5" - > - Nullam neque, ligula ac, id blandit. - </div> + ></div> </td> </tr> <tr @@ -438,9 +420,7 @@ class="contentText gr-diff" data-side="right" id="right-content-6" - > - Sagittis tincidunt torquent, tempor nunc amet. - </div> + ></div> </td> </tr> <tr @@ -477,9 +457,7 @@ class="contentText gr-diff" data-side="right" id="right-content-7" - > - At rhoncus id. - </div> + ></div> </td> </tr> </tbody> @@ -509,9 +487,7 @@ class="contentText gr-diff" data-side="left" id="left-content-5" - > - Sem nascetur, erat ut, non in. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="8"> <button @@ -530,9 +506,7 @@ class="contentText gr-diff" data-side="right" id="right-content-8" - > - Sem nascetur, erat ut, non in. - </div> + ></div> </td> </tr> <tr @@ -560,9 +534,7 @@ class="contentText gr-diff" data-side="left" id="left-content-6" - > - A donec, venenatis pellentesque dis. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="9"> <button @@ -581,9 +553,7 @@ class="contentText gr-diff" data-side="right" id="right-content-9" - > - A donec, venenatis pellentesque dis. - </div> + ></div> </td> </tr> <tr @@ -611,9 +581,7 @@ class="contentText gr-diff" data-side="left" id="left-content-7" - > - Mauris mauris. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="10"> <button @@ -632,9 +600,7 @@ class="contentText gr-diff" data-side="right" id="right-content-10" - > - Mauris mauris. - </div> + ></div> </td> </tr> <tr @@ -662,9 +628,7 @@ class="contentText gr-diff" data-side="left" id="left-content-8" - > - Quisque nisl duis, facilisis viverra. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="11"> <button @@ -683,9 +647,7 @@ class="contentText gr-diff" data-side="right" id="right-content-11" - > - Quisque nisl duis, facilisis viverra. - </div> + ></div> </td> </tr> <tr @@ -713,9 +675,7 @@ class="contentText gr-diff" data-side="left" id="left-content-9" - > - Justo purus, semper eget et. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="12"> <button @@ -734,9 +694,7 @@ class="contentText gr-diff" data-side="right" id="right-content-12" - > - Justo purus, semper eget et. - </div> + ></div> </td> </tr> </tbody> @@ -766,9 +724,7 @@ class="contentText gr-diff" data-side="left" id="left-content-10" - > - Est amet, vestibulum pellentesque. - </div> + ></div> </td> <td class="blankLineNum gr-diff right"></td> <td class="blank gr-diff no-intraline-info right sign"></td> @@ -805,9 +761,7 @@ class="contentText gr-diff" data-side="left" id="left-content-11" - > - Erat ligula. - </div> + ></div> </td> <td class="blankLineNum gr-diff right"></td> <td class="blank gr-diff no-intraline-info right sign"></td> @@ -844,9 +798,7 @@ class="contentText gr-diff" data-side="left" id="left-content-12" - > - Justo eros. - </div> + ></div> </td> <td class="blankLineNum gr-diff right"></td> <td class="blank gr-diff no-intraline-info right sign"></td> @@ -883,9 +835,7 @@ class="contentText gr-diff" data-side="left" id="left-content-13" - > - Fringilla quisque. - </div> + ></div> </td> <td class="blankLineNum gr-diff right"></td> <td class="blank gr-diff no-intraline-info right sign"></td> @@ -924,9 +874,7 @@ class="contentText gr-diff" data-side="left" id="left-content-14" - > - Arcu eget, rhoncus amet cursus, ipsum elementum. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="13"> <button @@ -945,9 +893,7 @@ class="contentText gr-diff" data-side="right" id="right-content-13" - > - Arcu eget, rhoncus amet cursus, ipsum elementum. - </div> + ></div> </td> </tr> <tr @@ -975,9 +921,7 @@ class="contentText gr-diff" data-side="left" id="left-content-15" - > - Eros suspendisse. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="14"> <button @@ -996,9 +940,7 @@ class="contentText gr-diff" data-side="right" id="right-content-14" - > - Eros suspendisse. - </div> + ></div> </td> </tr> </tbody> @@ -1028,10 +970,7 @@ class="contentText gr-diff" data-side="left" id="left-content-16" - > - Rhoncus tempor, ultricies - <hl class="gr-diff intraline"> aliquam </hl> ipsum. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="15"> <button @@ -1050,10 +989,7 @@ class="contentText gr-diff" data-side="right" id="right-content-15" - > - Rhoncus tempor, ultricies - <hl class="gr-diff intraline"> praesent </hl> ipsum. - </div> + ></div> </td> </tr> </tbody> @@ -1083,9 +1019,7 @@ class="contentText gr-diff" data-side="left" id="left-content-17" - > - Sollicitudin duis. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="16"> <button @@ -1104,9 +1038,7 @@ class="contentText gr-diff" data-side="right" id="right-content-16" - > - Sollicitudin duis. - </div> + ></div> </td> </tr> <tr @@ -1134,9 +1066,7 @@ class="contentText gr-diff" data-side="left" id="left-content-18" - > - Blandit blandit, ante nisl fusce. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="17"> <button @@ -1155,9 +1085,7 @@ class="contentText gr-diff" data-side="right" id="right-content-17" - > - Blandit blandit, ante nisl fusce. - </div> + ></div> </td> </tr> <tr @@ -1185,9 +1113,7 @@ class="contentText gr-diff" data-side="left" id="left-content-19" - > - Felis ac at, tellus consectetuer. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="18"> <button @@ -1206,9 +1132,7 @@ class="contentText gr-diff" data-side="right" id="right-content-18" - > - Felis ac at, tellus consectetuer. - </div> + ></div> </td> </tr> </tbody> @@ -1276,9 +1200,7 @@ class="contentText gr-diff" data-side="left" id="left-content-38" - > - Ullamcorper nunc ante, nec imperdiet felis, consectetur. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="37"> <button @@ -1297,9 +1219,7 @@ class="contentText gr-diff" data-side="right" id="right-content-37" - > - Ullamcorper nunc ante, nec imperdiet felis, consectetur. - </div> + ></div> </td> </tr> <tr @@ -1327,9 +1247,7 @@ class="contentText gr-diff" data-side="left" id="left-content-39" - > - Ac eget. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="38"> <button @@ -1348,9 +1266,7 @@ class="contentText gr-diff" data-side="right" id="right-content-38" - > - Ac eget. - </div> + ></div> </td> </tr> <tr @@ -1378,9 +1294,7 @@ class="contentText gr-diff" data-side="left" id="left-content-40" - > - Vel fringilla, interdum pellentesque placerat, proin ante. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="39"> <button @@ -1399,9 +1313,7 @@ class="contentText gr-diff" data-side="right" id="right-content-39" - > - Vel fringilla, interdum pellentesque placerat, proin ante. - </div> + ></div> </td> </tr> </tbody> @@ -1440,9 +1352,7 @@ class="contentText gr-diff" data-side="right" id="right-content-40" - > - Eu congue risus. - </div> + ></div> </td> </tr> <tr @@ -1479,9 +1389,7 @@ class="contentText gr-diff" data-side="right" id="right-content-41" - > - Enim ac, quis elementum. - </div> + ></div> </td> </tr> <tr @@ -1518,9 +1426,7 @@ class="contentText gr-diff" data-side="right" id="right-content-42" - > - Non et elit. - </div> + ></div> </td> </tr> <tr @@ -1557,9 +1463,7 @@ class="contentText gr-diff" data-side="right" id="right-content-43" - > - Etiam aliquam, diam vel nunc. - </div> + ></div> </td> </tr> </tbody> @@ -1589,9 +1493,7 @@ class="contentText gr-diff" data-side="left" id="left-content-41" - > - Nec at. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="44"> <button @@ -1610,9 +1512,7 @@ class="contentText gr-diff" data-side="right" id="right-content-44" - > - Nec at. - </div> + ></div> </td> </tr> <tr @@ -1640,9 +1540,7 @@ class="contentText gr-diff" data-side="left" id="left-content-42" - > - Arcu mauris, venenatis lacus fermentum, praesent duis. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="45"> <button @@ -1661,9 +1559,7 @@ class="contentText gr-diff" data-side="right" id="right-content-45" - > - Arcu mauris, venenatis lacus fermentum, praesent duis. - </div> + ></div> </td> </tr> <tr @@ -1691,9 +1587,7 @@ class="contentText gr-diff" data-side="left" id="left-content-43" - > - Pellentesque amet et, tellus duis. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="46"> <button @@ -1712,9 +1606,7 @@ class="contentText gr-diff" data-side="right" id="right-content-46" - > - Pellentesque amet et, tellus duis. - </div> + ></div> </td> </tr> <tr @@ -1742,9 +1634,7 @@ class="contentText gr-diff" data-side="left" id="left-content-44" - > - Ipsum arcu vitae, justo elit, sed libero tellus. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="47"> <button @@ -1763,9 +1653,7 @@ class="contentText gr-diff" data-side="right" id="right-content-47" - > - Ipsum arcu vitae, justo elit, sed libero tellus. - </div> + ></div> </td> </tr> <tr @@ -1793,9 +1681,7 @@ class="contentText gr-diff" data-side="left" id="left-content-45" - > - Metus rutrum euismod, vivamus sodales, vel arcu nisl. - </div> + ></div> </td> <td class="gr-diff lineNum right" data-value="48"> <button @@ -1814,15 +1700,16 @@ class="contentText gr-diff" data-side="right" id="right-content-48" - > - Metus rutrum euismod, vivamus sodales, vel arcu nisl. - </div> + ></div> </td> </tr> </tbody> </table> </div> - ` + `, + { + ignoreTags: ['gr-legacy-text'], + } ); }; });