Skip to content

Commit

Permalink
[FilesBreadcrumb] Escape "/" in path component for breadcrumb
Browse files Browse the repository at this point in the history
XfBreadcrumb uses "/" to split the path, before setting path of
breadcrumb component, we need to escape the "/" in the path
(e.g. Nexus/Pixel (MTP) => Nexus%2FPixel (MTP))
so it won't break the split logic.

Add a unit test to check breadcrumb_container. Note: breadcrumb
itself already has a unit test to check the unescape logic.

Demo: http://go/scrcast/NjUwODY2Nzc4NTQ0NTM3NnwwZWRlMzA2ZS1iYw
Bug: b:263042677
Test: browser_test --gtest_filter="*BreadcrumbContainer"
Change-Id: I40b744fa6db523f87059322b6d7ccb9d936913a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4113705
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Wenbo Jie <wenbojie@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084850}
  • Loading branch information
PinkyJie authored and Chromium LUCI CQ committed Dec 19, 2022
1 parent 995423c commit 2a0eaa1
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 1 deletion.
4 changes: 4 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_jstest.cc
Expand Up @@ -365,3 +365,7 @@ IN_PROC_BROWSER_TEST_F(FileManagerJsTest, XfIcon) {
IN_PROC_BROWSER_TEST_F(FileManagerJsTest, XfPathDisplay) {
RunTestURL("widgets/xf_path_display_unittest.js");
}

IN_PROC_BROWSER_TEST_F(FileManagerJsTest, BreadcrumbContainer) {
RunTestURL("containers/breadcrumb_container_unittest.js");
}
Expand Up @@ -81,7 +81,8 @@ export class BreadcrumbContainer {
this.container_.appendChild(breadcrumb);
}

const path = pathComponents.map(p => p.label).join('/');
const path =
pathComponents.map(p => p.label.replace(/\//g, '%2F')).join('/');
breadcrumb!.path = path;
this.currentFileKey_ = key;
this.pathKeys_ = pathComponents.map(p => p.key);
Expand Down
@@ -0,0 +1,68 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import {assertEquals, assertNotEquals} from 'chrome://webui-test/chai_assert.js';

import {CurrentDirectory, PropStatus} from '../externs/ts/state.js';
import {getEmptyState, getStore} from '../state/store.js';

import {BreadcrumbContainer} from './breadcrumb_container.js';


/** An instance of BreadcrumbContainer. */
let breadcrumbContainer: BreadcrumbContainer|null = null;

export function setUp() {
document.body.innerHTML = '<div id="breadcrumb-container"></div>';
breadcrumbContainer =
new BreadcrumbContainer(document.querySelector('#breadcrumb-container')!);
}

/**
* Tests that when current directory has "/" in the path, it will be escaped
* before passing to the underlying breadcrumb.
*/
export function testPathWithSlash(done: () => void) {
const store = getStore();
store.init(getEmptyState());
const currentDirectory: CurrentDirectory = {
key: 'filesystem:chrome://file-manager/external/aaa/bbb',
status: PropStatus.SUCCESS,
rootType: undefined,
pathComponents: [
{name: 'Nexus/Pixel(MTP)', label: 'Nexus/Pixel(MTP)', key: ''},
{name: 'DCIM', label: 'DCIM', key: ''},
],
content: {
keys: [],
},
selection: {
keys: [],
dirCount: 0,
fileCount: 0,
hostedCount: undefined,
offlineCachedCount: undefined,
fileTasks: {
policyDefaultHandlerStatus: undefined,
defaultTask: undefined,
tasks: [],
status: PropStatus.SUCCESS,
},
},
hasDlpDisabledFiles: false,
};

// Simulate a state change from the store.
breadcrumbContainer!.onStateChanged({...store.getState(), currentDirectory});

// Check that breadcrumb's path has been escaped.
const breadcrumb = document.querySelector('xf-breadcrumb');
assertNotEquals(null, breadcrumb);
assertEquals(
breadcrumb!.path,
'Nexus%2FPixel(MTP)/DCIM',
);

done();
}
1 change: 1 addition & 0 deletions ui/file_manager/file_names.gni
Expand Up @@ -310,6 +310,7 @@ ts_templates = [

ts_test_files = [
# Containers
"file_manager/containers/breadcrumb_container_unittest.ts",
"file_manager/containers/nudge_container_unittest.ts",

# Lib:
Expand Down

0 comments on commit 2a0eaa1

Please sign in to comment.