Commit f4fced86 authored by Himanshu Kapoor's avatar Himanshu Kapoor

Fix encoding issues when uploading

In Web IDE, when you upload a file, sometimes the encoding isn't
detected correctly. This change fixes fix it.

1. The changeset https://gitlab.com/gitlab-org/gitlab/commit/d1fb29e5
introduced the usage of `FileReader.readAsDataURL` for both binary and
text files. The resulting base64 content is then decoded using `atob`
function, which uses ASCII as its default encoding, whereas for
Unicode content, UTF-8 should be the default.
2. Improvements to the download viewer. Files whose mime type
cannot be detected will have their file name visible before the
download button, and in the name of the downloaded file. Previously,
this defaulted to the base64 uri and the file being named `download`.
parent c785530c
......@@ -43,21 +43,28 @@ export default {
},
createFile(target, file) {
const { name } = file;
let { result } = target;
const encodedContent = result.split('base64,')[1];
const encodedContent = target.result.split('base64,')[1];
const rawContent = encodedContent ? atob(encodedContent) : '';
const isText = this.isText(rawContent, file.type);
result = isText ? rawContent : encodedContent;
const emitCreateEvent = content =>
this.$emit('create', {
name: `${this.path ? `${this.path}/` : ''}${name}`,
type: 'blob',
content,
base64: !isText,
binary: !isText,
rawPath: !isText ? target.result : '',
});
this.$emit('create', {
name: `${this.path ? `${this.path}/` : ''}${name}`,
type: 'blob',
content: result,
base64: !isText,
binary: !isText,
rawPath: !isText ? target.result : '',
});
if (isText) {
const reader = new FileReader();
reader.addEventListener('load', e => emitCreateEvent(e.target.result), { once: true });
reader.readAsText(file);
} else {
emitCreateEvent(encodedContent);
}
},
readFile(file) {
const reader = new FileReader();
......
......@@ -13,6 +13,11 @@ export default {
type: String,
required: true,
},
filePath: {
type: String,
required: false,
default: '',
},
fileSize: {
type: Number,
required: false,
......@@ -24,7 +29,8 @@ export default {
return numberToHumanSize(this.fileSize);
},
fileName() {
return this.path.split('/').pop();
// path could be a base64 uri too, so check if filePath was passed additionally
return (this.filePath || this.path).split('/').pop();
},
},
};
......@@ -39,7 +45,13 @@ export default {
({{ fileSizeReadable }})
</template>
</p>
<gl-link :href="path" class="btn btn-default" rel="nofollow" download target="_blank">
<gl-link
:href="path"
class="btn btn-default"
rel="nofollow"
:download="fileName"
target="_blank"
>
<icon :size="16" name="download" class="float-left append-right-8" />
{{ __('Download') }}
</gl-link>
......
---
title: Fix some of the file encoding issues when uploading in the Web IDE
merge_request: 23761
author:
type: fixed
......@@ -14,7 +14,7 @@ describe('new dropdown upload', () => {
vm.entryName = 'testing';
spyOn(vm, '$emit');
spyOn(vm, '$emit').and.callThrough();
});
afterEach(() => {
......@@ -61,31 +61,44 @@ describe('new dropdown upload', () => {
const binaryTarget = {
result: 'base64,w4I=',
};
const textFile = {
name: 'textFile',
type: 'text/plain',
};
const textFile = new File(['plain text'], 'textFile');
const binaryFile = {
name: 'binaryFile',
type: 'image/png',
};
it('creates file in plain text (without encoding) if the file content is plain text', () => {
beforeEach(() => {
spyOn(FileReader.prototype, 'readAsText').and.callThrough();
});
it('calls readAsText and creates file in plain text (without encoding) if the file content is plain text', done => {
const waitForCreate = new Promise(resolve => vm.$on('create', resolve));
vm.createFile(textTarget, textFile);
expect(vm.$emit).toHaveBeenCalledWith('create', {
name: textFile.name,
type: 'blob',
content: 'plain text',
base64: false,
binary: false,
rawPath: '',
});
expect(FileReader.prototype.readAsText).toHaveBeenCalledWith(textFile);
waitForCreate
.then(() => {
expect(vm.$emit).toHaveBeenCalledWith('create', {
name: textFile.name,
type: 'blob',
content: 'plain text',
base64: false,
binary: false,
rawPath: '',
});
})
.then(done)
.catch(done.fail);
});
it('splits content on base64 if binary', () => {
vm.createFile(binaryTarget, binaryFile);
expect(FileReader.prototype.readAsText).not.toHaveBeenCalledWith(textFile);
expect(vm.$emit).toHaveBeenCalledWith('create', {
name: binaryFile.name,
type: 'blob',
......
......@@ -58,14 +58,34 @@ describe('ContentViewer', () => {
it('renders fallback download control', done => {
createComponent({
path: 'test.abc',
path: 'somepath/test.abc',
fileSize: 1024,
});
setTimeout(() => {
expect(vm.$el.querySelector('.file-info').textContent.trim()).toContain('test.abc');
expect(vm.$el.querySelector('.file-info').textContent.trim()).toContain('(1.00 KiB)');
expect(vm.$el.querySelector('.btn.btn-default').textContent.trim()).toContain('Download');
expect(
vm.$el
.querySelector('.file-info')
.textContent.trim()
.replace(/\s+/, ' '),
).toEqual('test.abc (1.00 KiB)');
expect(vm.$el.querySelector('.btn.btn-default').textContent.trim()).toEqual('Download');
done();
});
});
it('renders fallback download control for file with a data URL path properly', done => {
createComponent({
path: 'data:application/octet-stream;base64,U0VMRUNUICfEhHNnc2cnIGZyb20gVGFibGVuYW1lOwoK',
filePath: 'somepath/test.abc',
});
setTimeout(() => {
expect(vm.$el.querySelector('.file-info').textContent.trim()).toEqual('test.abc');
expect(vm.$el.querySelector('.btn.btn-default')).toHaveAttr('download', 'test.abc');
expect(vm.$el.querySelector('.btn.btn-default').textContent.trim()).toEqual('Download');
done();
});
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment