Fix binary diff
One issue was that thread were not displayed anymore. The root cause
for that is that we moved the creation of thread groups into the diff
builders, but the binary diff builder was then *replacing* the content
of the table cell with the text `Difference in binary files`, thus
overriding the `thread-group` div that had been there before.
We also fix another bug: Ever since introducing the LOST line number
we have been rendering two FILE diff rows for binary diffs, because the
builder was relying on being called just once.
Release-Notes: skip
Google-Bug-Id: b/132206094
Change-Id: I99bd353d8aaae536b32ad184340e655f640bccec
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
index c095ffb..5267f307 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
@@ -5,9 +5,12 @@
*/
import {GrDiffBuilderUnified} from './gr-diff-builder-unified';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
-import {queryAndAssert} from '../../../utils/common-util';
import {createElementDiff} from '../gr-diff/gr-diff-utils';
+import {GrDiffGroup} from '../gr-diff/gr-diff-group';
+import {GrDiffLine} from '../gr-diff/gr-diff-line';
+import {GrDiffLineType} from '../../../api/diff';
+import {queryAndAssert} from '../../../utils/common-util';
+import {html, render} from 'lit';
export class GrDiffBuilderBinary extends GrDiffBuilderUnified {
constructor(
@@ -18,12 +21,20 @@
super(diff, prefs, outputEl);
}
- override buildSectionElement(): HTMLElement {
+ override buildSectionElement(group: GrDiffGroup): HTMLElement {
const section = createElementDiff('tbody', 'binary-diff');
+ // Do not create a diff row for 'LOST'.
+ if (group.lines[0].beforeNumber !== 'FILE') return section;
+
const line = new GrDiffLine(GrDiffLineType.BOTH, 'FILE', 'FILE');
const fileRow = this.createRow(line);
- const contentTd = queryAndAssert(fileRow, 'td.both.file')!;
- contentTd.textContent = ' Difference in binary files';
+ const contentTd = queryAndAssert<HTMLTableCellElement>(
+ fileRow,
+ 'td.both.file'
+ )!;
+ const div = document.createElement('div');
+ render(html`<span>Difference in binary files</span>`, div);
+ contentTd.insertBefore(div, contentTd.firstChild);
section.appendChild(fileRow);
return section;
}
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 650ff78..0dc1e12 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
@@ -1870,6 +1870,78 @@
assert.isTrue(container.classList.contains('displayLine'));
});
+ suite('binary diffs', () => {
+ test('render binary diff', async () => {
+ element.prefs = {
+ ...MINIMAL_PREFS,
+ };
+ element.diff = {
+ meta_a: {name: 'carrot.exe', content_type: 'binary', lines: 0},
+ meta_b: {name: 'carrot.exe', content_type: 'binary', lines: 0},
+ change_type: 'MODIFIED',
+ intraline_status: 'OK',
+ diff_header: [],
+ content: [],
+ binary: true,
+ };
+ await waitForEventOnce(element, 'render');
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <div class="diffContainer sideBySide">
+ <table class="selected-right" id="diffTable">
+ <colgroup>
+ <col class="blame gr-diff" />
+ <col width="48" />
+ <col width="48" />
+ <col />
+ </colgroup>
+ <tbody class="binary-diff gr-diff"></tbody>
+ <tbody class="binary-diff gr-diff">
+ <tr class="both diff-row gr-diff unified" tabindex="-1">
+ <td class="blame gr-diff" data-line-number="FILE"></td>
+ <td class="gr-diff left lineNum" data-value="FILE">
+ <button
+ aria-label="Add file comment"
+ class="gr-diff left lineNumButton"
+ data-value="FILE"
+ id="left-button-FILE"
+ tabindex="-1"
+ >
+ File
+ </button>
+ </td>
+ <td class="gr-diff lineNum right" data-value="FILE">
+ <button
+ aria-label="Add file comment"
+ class="gr-diff lineNumButton right"
+ data-value="FILE"
+ id="right-button-FILE"
+ tabindex="-1"
+ >
+ File
+ </button>
+ </td>
+ <td
+ class="both content file gr-diff no-intraline-info right"
+ >
+ <div>
+ <span> Difference in binary files </span>
+ </div>
+ <div class="thread-group" data-side="right">
+ <slot name="right-FILE"> </slot>
+ </div>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ </div>
+ `
+ );
+ });
+ });
+
suite('image diffs', () => {
let mockFile1: ImageInfo;
let mockFile2: ImageInfo;
