Merge branch 'ph/fixBranchSlash' into 'master'

Fixes file browser with slashes in branch name

Closes #199308 and #215062

See merge request gitlab-org/gitlab!29967

app/assets/javascripts/repository/components/breadcrumbs.vue

@@ -108,14 +108,14 @@ export default {
             return acc.concat({
               name,
               path,
-              to: `/-/tree/${joinPaths(encodeURIComponent(this.ref), path)}`,
+              to: `/-/tree/${joinPaths(escapeFileUrl(this.ref), path)}`,
             });
           },
           [
             {
               name: this.projectShortPath,
               path: '/',
-              to: `/-/tree/${encodeURIComponent(this.ref)}/`,
+              to: `/-/tree/${escapeFileUrl(this.ref)}/`,
             },
           ],
         );

app/assets/javascripts/repository/components/table/parent_row.vue

 <script>
 import { GlLoadingIcon } from '@gitlab/ui';
+import { escapeFileUrl } from '~/lib/utils/url_utility';
 
 export default {
   components: {
@@ -28,7 +29,7 @@ export default {
       return splitArray.map(p => encodeURIComponent(p)).join('/');
     },
     parentRoute() {
-      return { path: `/-/tree/${encodeURIComponent(this.commitRef)}/${this.parentPath}` };
+      return { path: `/-/tree/${escapeFileUrl(this.commitRef)}/${this.parentPath}` };
     },
   },
   methods: {

app/assets/javascripts/repository/components/table/row.vue

@@ -99,7 +99,7 @@ export default {
   computed: {
     routerLinkTo() {
       return this.isFolder
-        ? { path: `/-/tree/${encodeURIComponent(this.ref)}/${escapeFileUrl(this.path)}` }
+        ? { path: `/-/tree/${escapeFileUrl(this.ref)}/${escapeFileUrl(this.path)}` }
         : null;
     },
     isFolder() {

app/assets/javascripts/repository/router.js

@@ -12,7 +12,7 @@ export default function createRouter(base, baseRef) {
     base: joinPaths(gon.relative_url_root || '', base),
     routes: [
       {
-        path: `(/-)?/tree/(${encodeURIComponent(baseRef)}|${baseRef})/:path*`,
+        path: `(/-)?/tree/(${encodeURIComponent(baseRef).replace(/%2F/g, '/')}|${baseRef})/:path*`,
         name: 'treePath',
         component: TreePage,
         props: route => ({

spec/features/projects/files/user_browses_files_spec.rb

@@ -180,6 +180,20 @@ describe "User browses files" do
       expect(page).to have_content("VERSION")
         .and have_content(".gitignore")
         .and have_content("LICENSE")
+
+      click_link("files")
+
+      page.within('.repo-breadcrumb') do
+        expect(page).to have_link('files')
+      end
+
+      click_link("html")
+
+      page.within('.repo-breadcrumb') do
+        expect(page).to have_link('html')
+      end
+
+      expect(page).to have_link('500.html')
     end
   end
 
@@ -193,6 +207,20 @@ describe "User browses files" do
       expect(page).to have_content("VERSION")
         .and have_content(".gitignore")
         .and have_content("LICENSE")
+
+      click_link("files")
+
+      page.within('.repo-breadcrumb') do
+        expect(page).to have_link('files')
+      end
+
+      click_link("html")
+
+      page.within('.repo-breadcrumb') do
+        expect(page).to have_link('html')
+      end
+
+      expect(page).to have_link('500.html')
     end
   end
 

spec/frontend/repository/router_spec.js

@@ -4,14 +4,15 @@ import createRouter from '~/repository/router';
 
 describe('Repository router spec', () => {
   it.each`
-    path                           | component    | componentName
-    ${'/'}                         | ${IndexPage} | ${'IndexPage'}
-    ${'/tree/master'}              | ${TreePage}  | ${'TreePage'}
-    ${'/-/tree/master'}            | ${TreePage}  | ${'TreePage'}
-    ${'/-/tree/master/app/assets'} | ${TreePage}  | ${'TreePage'}
-    ${'/-/tree/123/app/assets'}    | ${null}      | ${'null'}
-  `('sets component as $componentName for path "$path"', ({ path, component }) => {
-    const router = createRouter('', 'master');
+    path                                     | branch              | component    | componentName
+    ${'/'}                                   | ${'master'}         | ${IndexPage} | ${'IndexPage'}
+    ${'/tree/master'}                        | ${'master'}         | ${TreePage}  | ${'TreePage'}
+    ${'/-/tree/master'}                      | ${'master'}         | ${TreePage}  | ${'TreePage'}
+    ${'/-/tree/master/app/assets'}           | ${'master'}         | ${TreePage}  | ${'TreePage'}
+    ${'/-/tree/feature/test-%23/app/assets'} | ${'feature/test-#'} | ${TreePage}  | ${'TreePage'}
+    ${'/-/tree/123/app/assets'}              | ${'master'}         | ${null}      | ${'null'}
+  `('sets component as $componentName for path "$path"', ({ path, component, branch }) => {
+    const router = createRouter('', branch);
 
     const componentsForRoute = router.getMatchedComponents(path);