Make diff URLs pretty again 💐

Bug: Issue 4577
Change-Id: Ica06e4d21340f33235408d7ac7b431719f4ea077
This commit is contained in:
Andrew Bonventre
2016-09-18 12:54:26 -04:00
parent b2b65b0e04
commit 44eefb5e79
3 changed files with 19 additions and 5 deletions

View File

@@ -383,12 +383,16 @@
_computeDiffURL: function(changeNum, patchRange, path) {
// @see Issue 4255 regarding double-encoding.
path = encodeURIComponent(encodeURIComponent(path));
// @see Issue 4577 regarding more readable URLs.
path = path.replace(/%252F/g, '/');
path = path.replace(/%2520/g, '+');
return '/c/' +
encodeURIComponent(changeNum) +
'/' +
encodeURIComponent(this._patchRangeStr(patchRange)) +
'/' +
encodeURIComponent(encodeURIComponent(path));
path;
},
_patchRangeStr: function(patchRange) {

View File

@@ -39,6 +39,7 @@ limitations under the License.
setup(function() {
stub('gr-rest-api-interface', {
getLoggedIn: function() { return Promise.resolve(true); },
getPreferences: function() { return Promise.resolve({}); },
});
element = fixture('basic');
});
@@ -312,9 +313,9 @@ limitations under the License.
diffStub.restore();
});
test('file name should be double-escaped', function() {
test('path should be properly escaped', function() {
element._files = [
{__path: 'my+file.txt'},
{__path: 'foo bar/my+file.txt%'},
];
element.changeNum = '42';
element.patchRange = {
@@ -322,8 +323,12 @@ limitations under the License.
patchNum: '2',
};
flushAsynchronousOperations();
// Slashes should be preserved, and spaces should be translated to `+`.
// @see Issue 4255 regarding double-encoding.
// @see Issue 4577 regarding more readable URLs.
assert.equal(
element.$$('a').getAttribute('href'), '/c/42/2/my%252Bfile.txt');
element.$$('a').getAttribute('href'),
'/c/42/2/foo+bar/my%252Bfile.txt%2525');
});
});
</script>

View File

@@ -135,12 +135,17 @@
// Don't allow diffing the same patch number against itself.
if (params.basePatchNum === params.patchNum) {
// @see Issue 4255 regarding double-encoding.
var path = encodeURIComponent(encodeURIComponent(path));
// @see Issue 4577 regarding more readable URLs.
path = path.replace(/%252F/g, '/');
path = path.replace(/%2520/g, '+');
page.redirect('/c/' +
encodeURIComponent(params.changeNum) +
'/' +
encodeURIComponent(params.patchNum) +
'/' +
encodeURIComponent(encodeURIComponent(params.path)));
path);
return;
}