Add support for images in diffs
If gr-diff recognizes that the file difference it's representing is
between images, it uses a different diff-builder that displays images
in a side-by-side-manner. In this case gr-diff will also make requests
for the image data itself, which it can pass down into the image-based
diff-builder.
Adds methods to gr-rest-api-interface to support rendering the data
relevant to image diffs. For images that are revisions of the current
change, provides "getChangeFileContents". For images that come from the
parent tree (i.e. if the basePatchNum is "PARENT") the interface
provides "getCommitInfo" to determine the SHA of the parent commit, and
"getCommitFileContents" which can get file contents for a given commit.
Bug: Issue 3822
Change-Id: I9be025b4e549fca97c87cdbeede6cb64dea5eac0
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 3175517..da40a9d 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -272,6 +272,7 @@
</div>
</section>
<gr-file-list id="fileList"
+ change="[[_change]]"
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
comments="[[_comments]]"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 976cf9b..f15c654 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -170,6 +170,8 @@
</div>
</div>
<gr-diff hidden
+ project="[[change.project]]"
+ commit="[[change.current_revision]]"
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
path="[[file.__path]]"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 3257469..15f835d 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -36,6 +36,7 @@
type: Object,
value: function() { return document.body; },
},
+ change: Object,
_files: {
type: Array,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 0e99a15..903eabf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -174,12 +174,16 @@
available-patches="[[_computeAvailablePatches(_change.revisions)]]">
</gr-patch-range-select>
<div>
- <select id="modeSelect" on-change="_handleModeChange">
+ <select
+ id="modeSelect"
+ on-change="_handleModeChange"
+ hidden$="[[_computeModeSelectHidden(_isImageDiff)]]">
<option value="SIDE_BY_SIDE">Side By Side</option>
<option value="UNIFIED_DIFF">Unified</option>
</select>
<span hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]">
- /
+ <span
+ hidden$="[[_computeModeSelectHidden(_isImageDiff)]]">/</span>
<gr-button link
class="prefsButton"
on-tap="_handlePrefsTap">Preferences</gr-button>
@@ -193,6 +197,9 @@
on-cancel="_handlePrefsCancel"></gr-diff-preferences>
</gr-overlay>
<gr-diff id="diff"
+ project="[[_change.project]]"
+ commit="[[_change.current_revision]]"
+ is-image-diff="{{_isImageDiff}}"
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
path="[[_path]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index c75cb72..17aa0c4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -74,6 +74,7 @@
type: String,
computed: '_getDiffViewMode(changeViewState.diffMode, _userPrefs)'
},
+ _isImageDiff: Boolean,
},
behaviors: [
@@ -293,11 +294,11 @@
this._userPrefs = prefs;
}.bind(this)));
- promises.push(this.$.diff.reload());
+ promises.push(this._getChangeDetail(this._changeNum));
- Promise.all(promises).then(function() {
- this._loading = false;
- }.bind(this));
+ Promise.all(promises)
+ .then(function() { return this.$.diff.reload(); }.bind(this))
+ .then(function() { this._loading = false; }.bind(this));
},
_pathChanged: function(path) {
@@ -462,5 +463,9 @@
this.$.modeSelect.value = mode;
}
},
+
+ _computeModeSelectHidden: function() {
+ return this._isImageDiff;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js
new file mode 100644
index 0000000..b897708
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js
@@ -0,0 +1,100 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the 'License');
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an 'AS IS' BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+(function(window, GrDiffBuilderSideBySide) {
+ 'use strict';
+
+ function GrDiffBuilderImage(diff, comments, prefs, outputEl, baseImage,
+ revisionImage) {
+ GrDiffBuilderSideBySide.call(this, diff, comments, prefs, outputEl);
+ this._baseImage = baseImage;
+ this._revisionImage = revisionImage;
+ }
+
+ GrDiffBuilderImage.prototype = Object.create(
+ GrDiffBuilderSideBySide.prototype);
+ GrDiffBuilderImage.prototype.constructor = GrDiffBuilderImage;
+
+ GrDiffBuilderImage.prototype.emitDiff = function() {
+ this.emitGroup(this._groups[0]);
+
+ var section = this._createElement('tbody', 'image-diff');
+
+ this._emitImagePair(section);
+ this._emitImageLabels(section);
+
+ this._outputEl.appendChild(section);
+ };
+
+ GrDiffBuilderImage.prototype._emitImagePair = function(section) {
+ var tr = this._createElement('tr');
+
+ tr.appendChild(this._createElement('td'));
+ tr.appendChild(this._createImageCell(this._baseImage, 'left'));
+
+ tr.appendChild(this._createElement('td'));
+ tr.appendChild(this._createImageCell(this._revisionImage, 'right'));
+
+ section.appendChild(tr);
+ };
+
+ GrDiffBuilderImage.prototype._createImageCell = function(image, className) {
+ var td = this._createElement('td', className);
+ if (image) {
+ var imageEl = this._createElement('img');
+ imageEl.src = 'data:' + image.type + ';base64, ' + image.body;
+ image._height = imageEl.naturalHeight;
+ image._width = imageEl.naturalWidth;
+ imageEl.addEventListener('error', function(e) {
+ imageEl.remove();
+ td.textContent = '[Image failed to load]';
+ });
+ td.appendChild(imageEl);
+ }
+ return td;
+ };
+
+ GrDiffBuilderImage.prototype._emitImageLabels = function(section) {
+ var tr = this._createElement('tr');
+
+ tr.appendChild(this._createElement('td'));
+ var td = this._createElement('td', 'left');
+ var label = this._createElement('label');
+ label.textContent = this._getImageLabel(this._baseImage);
+ td.appendChild(label);
+ tr.appendChild(td);
+
+ tr.appendChild(this._createElement('td'));
+ td = this._createElement('td', 'right');
+ label = this._createElement('label');
+ label.textContent = this._getImageLabel(this._revisionImage);
+ td.appendChild(label);
+ tr.appendChild(td);
+
+ section.appendChild(tr);
+ };
+
+ GrDiffBuilderImage.prototype._getImageLabel = function(image) {
+ if (image) {
+ var type = image.type || image._expectedType;
+ if (image._width && image._height) {
+ return image._width + '⨉' + image._height + ' ' + type;
+ } else {
+ return type;
+ }
+ }
+ return 'No image';
+ };
+
+ window.GrDiffBuilderImage = GrDiffBuilderImage;
+})(window, GrDiffBuilderSideBySide);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
index c2197eb..f55037c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
@@ -15,6 +15,7 @@
'use strict';
function GrDiffBuilder(diff, comments, prefs, outputEl) {
+ this._diff = diff;
this._comments = comments;
this._prefs = prefs;
this._outputEl = outputEl;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index e4603b8..826a21e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -43,6 +43,17 @@
.section {
background-color: #eee;
}
+ .image-diff .gr-diff {
+ text-align: center;
+ }
+ .image-diff img {
+ max-width: 50em;
+ outline: 1px solid #ccc;
+ }
+ .image-diff label {
+ font-family: var(--font-family);
+ font-style: italic;
+ }
.diff-row.target-row.target-side-left .lineNum.left,
.diff-row.target-row.target-side-right .lineNum.right,
.diff-row.target-row.unified .lineNum {
@@ -151,5 +162,6 @@
<script src="gr-diff-builder.js"></script>
<script src="gr-diff-builder-side-by-side.js"></script>
<script src="gr-diff-builder-unified.js"></script>
+ <script src="gr-diff-builder-image.js"></script>
<script src="gr-diff.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 660b86f..4cbe10f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -42,6 +42,13 @@
type: Object,
observer: '_projectConfigChanged',
},
+ project: String,
+ commit: String,
+ isImageDiff: {
+ type: Boolean,
+ computed: '_computeIsImageDiff(_diff)',
+ notify: true,
+ },
_loggedIn: {
type: Boolean,
@@ -82,6 +89,7 @@
promises.push(this._getDiff().then(function(diff) {
this._diff = diff;
+ return this._loadDiffAssets();
}.bind(this)));
promises.push(this._getDiffCommentsAndDrafts().then(function(comments) {
@@ -414,8 +422,40 @@
return this.$.restAPI.getLoggedIn();
},
+ _computeIsImageDiff: function() {
+ if (!this._diff) { return false; }
+
+ var isA = this._diff.meta_a &&
+ this._diff.meta_a.content_type.indexOf('image/') === 0;
+ var isB = this._diff.meta_b &&
+ this._diff.meta_b.content_type.indexOf('image/') === 0;
+
+ return this._diff.binary && (isA || isB);
+ },
+
+ _loadDiffAssets: function() {
+ if (this.isImageDiff) {
+ return this._getImages().then(function(images) {
+ this._baseImage = images.baseImage;
+ this._revisionImage = images.revisionImage;
+ }.bind(this));
+ } else {
+ this._baseImage = null;
+ this._revisionImage = null;
+ return Promise.resolve();
+ }
+ },
+
+ _getImages: function() {
+ return this.$.restAPI.getImagesForDiff(this.project, this.commit,
+ this.changeNum, this._diff, this.patchRange);
+ },
+
_getDiffBuilder: function(diff, comments, prefs) {
- if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
+ if (this.isImageDiff) {
+ return new GrDiffBuilderImage(diff, comments, prefs, this.$.diffTable,
+ this._baseImage, this._revisionImage);
+ } else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
return new GrDiffBuilderSideBySide(diff, comments, prefs,
this.$.diffTable);
} else if (this.viewMode === DiffViewMode.UNIFIED) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index f02b861..cb091c9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -20,6 +20,7 @@
<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<script src="../../../scripts/util.js"></script>
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="gr-diff.html">
@@ -242,5 +243,105 @@
],
}));
});
+
+ test('renders image diffs', function(done) {
+ var mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ var mockFile1 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAEwsAAA' +
+ 'AAAAAAAAAAAAAA/w==',
+ type: 'image/bmp',
+ };
+ var mockFile2 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAEwsAAA' +
+ 'AAAAAAAAAA/////w==',
+ type: 'image/bmp'
+ };
+ var mockCommit = {
+ commit: '9a1a1d10baece5efbba10bc4ccf808a67a50ac0a',
+ parents: [{
+ commit: '7338aa9adfe57909f1fdaf88975cdea467d3382f',
+ subject: 'Added a carrot',
+ }],
+ author: {
+ name: 'Wyatt Allen',
+ email: 'wyatta@google.com',
+ date: '2016-05-23 21:44:51.000000000',
+ tz: -420,
+ },
+ committer: {
+ name: 'Wyatt Allen',
+ email: 'wyatta@google.com',
+ date: '2016-05-25 00:25:41.000000000',
+ tz: -420,
+ },
+ subject: 'Updated the carrot',
+ message: 'Updated the carrot\n\nChange-Id: Iabcd123\n',
+ };
+ var mockComments = {baseComments: [], comments: []};
+
+ var stubs = [];
+ stubs.push(sinon.stub(element, '_getDiff',
+ function() { return Promise.resolve(mockDiff); }));
+ stubs.push(sinon.stub(element.$.restAPI, 'getCommitInfo',
+ function() { return Promise.resolve(mockCommit); }));
+ stubs.push(sinon.stub(element.$.restAPI,
+ 'getCommitFileContents',
+ function() { return Promise.resolve(mockFile1); }));
+ stubs.push(sinon.stub(element.$.restAPI,
+ 'getChangeFileContents',
+ function() { return Promise.resolve(mockFile2); }));
+ stubs.push(sinon.stub(element.$.restAPI, '_getDiffComments',
+ function() { return Promise.resolve(mockComments); }));
+ stubs.push(sinon.stub(element.$.restAPI, 'getDiffDrafts',
+ function() { return Promise.resolve(mockComments); }));
+
+ element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
+
+ var rendered = function() {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(element._getDiffBuilder(element._diff,
+ element._comments, element.prefs), GrDiffBuilderImage);
+
+ // The left image rendered with the parent commit's version of the file.
+ var leftInmage = element.$.diffTable.querySelector('td.left img');
+ assert.isOk(leftInmage);
+ assert.equal(leftInmage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile1.body);
+
+ // The right image rendered with this change's revision of the image.
+ var rightInmage = element.$.diffTable.querySelector('td.right img');
+ assert.isOk(rightInmage);
+ assert.equal(rightInmage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile2.body);
+
+ // Cleanup.
+ element.removeEventListener('render', rendered);
+ stubs.forEach(function(stub) { stub.restore(); });
+
+ done();
+ };
+
+ element.addEventListener('render', rendered);
+
+ element.$.restAPI.getDiffPreferences().then(function(prefs) {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
index 10c8a29..72ae4e4 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
@@ -18,7 +18,6 @@
<script src="../../../bower_components/fetch/fetch.js"></script>
<dom-module id="gr-rest-api-interface">
- <template></template>
<script src="gr-rest-api-interface.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index e681bf9..9604ded 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -682,5 +682,84 @@
return '';
},
+ getCommitInfo: function(project, commit) {
+ return this.fetchJSON(
+ '/projects/' + encodeURIComponent(project) +
+ '/commits/' + encodeURIComponent(commit));
+ },
+
+ _fetchB64File: function(url) {
+ return fetch(url).then(function(response) {
+ var type = response.headers.get('X-FYI-Content-Type');
+ return response.text()
+ .then(function(text) {
+ return {body: text, type: type};
+ });
+ });
+ },
+
+ getChangeFileContents: function(changeId, patchNum, path) {
+ return this._fetchB64File(
+ '/changes/' + encodeURIComponent(changeId) +
+ '/revisions/' + encodeURIComponent(patchNum) +
+ '/files/' + encodeURIComponent(path) +
+ '/content');
+ },
+
+ getCommitFileContents: function(projectName, commit, path) {
+ return this._fetchB64File(
+ '/projects/' + encodeURIComponent(projectName) +
+ '/commits/' + encodeURIComponent(commit) +
+ '/files/' + encodeURIComponent(path) +
+ '/content');
+ },
+
+ getImagesForDiff: function(project, commit, changeNum, diff, patchRange) {
+ var promiseA;
+ var promiseB;
+
+ if (diff.meta_a && diff.meta_a.content_type.indexOf('image/') === 0) {
+ if (patchRange.basePatchNum === 'PARENT') {
+ // Need the commit info know the parent SHA.
+ promiseA = this.getCommitInfo(project, commit).then(function(info) {
+ if (info.parents.length !== 1) {
+ return Promise.reject('Change commit has multiple parents.');
+ }
+ var parent = info.parents[0].commit;
+ return this.getCommitFileContents(project, parent,
+ diff.meta_a.name);
+ }.bind(this));
+
+ } else {
+ promiseA = this.getChangeFileContents(changeNum,
+ patchRange.basePatchNum, diff.meta_a.name);
+ }
+ } else {
+ promiseA = Promise.resolve(null);
+ }
+
+ if (diff.meta_b && diff.meta_b.content_type.indexOf('image/') === 0) {
+ promiseB = this.getChangeFileContents(changeNum, patchRange.patchNum,
+ diff.meta_b.name);
+ } else {
+ promiseB = Promise.resolve(null);
+ }
+
+ return Promise.all([promiseA, promiseB])
+ .then(function(results) {
+ var baseImage = results[0];
+ var revisionImage = results[1];
+
+ // Sometimes the server doesn't send back the content type.
+ if (baseImage) {
+ baseImage._expectedType = diff.meta_a.content_type;
+ }
+ if (revisionImage) {
+ revisionImage._expectedType = diff.meta_b.content_type;
+ }
+
+ return {baseImage: baseImage, revisionImage: revisionImage};
+ }.bind(this));
+ },
});
})();
