Merge branch '26113-webide-upload-encoding' into 'master'

Web IDE: Fix encoding issues when uploading

See merge request gitlab-org/gitlab!23761

app/assets/javascripts/ide/components/new_dropdown/upload.vue

@@ -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();

app/assets/javascripts/vue_shared/components/content_viewer/viewers/download_viewer.vue

@@ -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>

changelogs/unreleased/26113-webide-upload-encoding.yml

+---
+title: Fix some of the file encoding issues when uploading in the Web IDE
+merge_request: 23761
+author:
+type: fixed

spec/javascripts/ide/components/new_dropdown/upload_spec.js

@@ -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',

spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js

@@ -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();
     });