Commit 1088890e authored by Noel Gordon's avatar Noel Gordon Committed by Commit Bot

[breadcrumb] Escape location line component '/' characters

A volume label can be the first part of a breadcrumb path - that label
can contain a '/' which the breadcrumb uses as the path separator when
splitting a path into it parts (components).

Volume labels like "Nexus/Pixel (MTP)" break the breadcrumb's '/' path
splitter (see bug).

Escape the path '/' splitter in location line, before sending the path
to the breadcrumb. The breadcrumb can then correctly split the path on
'/' and compute its parts. When presenting the parts in the breadcrumb
UI, window.unescape() them first to undo location line escaping.

Add a new unit test fixture testBreadcrumbRendersEscapedPathParts() to
test the breadcrumb path character unescaping behavior.

Bug: 1094105
Change-Id: I25f7622d31a72458412a0a5271ffb3aa1d5b69ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317246Reviewed-by: default avatarAlex Danilo <adanilo@chromium.org>
Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791745}
parent 4d7a7630
......@@ -267,7 +267,7 @@ class BreadCrumb extends HTMLElement {
}
buttons[i].removeAttribute('has-tooltip');
buttons[i].textContent = text;
buttons[i].textContent = window.unescape(text);
buttons[i].hidden = !text;
buttons[i].disabled = false;
!!text && enabled.push(i);
......@@ -310,7 +310,7 @@ class BreadCrumb extends HTMLElement {
let elidedParts = '';
for (let i = 1; i < parts.length - 2; ++i) {
elidedParts += `<button class='dropdown-item'>${
parts[i]}<paper-ripple></paper-ripple></button>`;
window.unescape(parts[i])}<paper-ripple></paper-ripple></button>`;
}
const menu = this.shadowRoot.querySelector('cr-action-menu');
......
......@@ -329,6 +329,40 @@ function testBreadcrumbMoreThanFourElementPathsElide() {
assertEquals(expect, path + ' ' + getBreadCrumbButtonState());
}
/**
* Tests rendering a path where the path parts have escaped characters. Again,
* the elider should be visible (not hidden and have display) because the path
* has more than four parts.
*
* The drop-down menu button should contain the elided path parts and can have
* display, but are invisible because the elider drop-down menu is closed.
*/
function testBreadcrumbRendersEscapedPathParts() {
const element = getBreadCrumb();
// Set path.
element.path = 'A%2FA/B%2FB/C %2F/%2FD /%2F%2FE/Nexus%2FPixel %28MTP%29';
// Elider button drop-down menu should be in the 'closed' state.
const elider = getBreadCrumbEliderButton();
assertEquals('false', elider.getAttribute('aria-expanded'));
// clang-format off
const expect = element.path +
' 1: display:block id=first text=[A/A]' +
' 2: display:flex elider[aria-expanded=false,aria-haspopup,aria-label]' +
' dropdown-item: display:block text=[B/B]' +
' dropdown-item: display:block text=[C /]' +
' dropdown-item: display:block text=[/D ]' +
' 3: display:none id=second hidden' +
' 4: display:block id=third text=[//E]' +
' 5: display:block id=fourth text=[Nexus/Pixel (MTP)]';
// clang-format on
const path = element.parts.join('/');
assertEquals(expect, path + ' ' + getBreadCrumbButtonState());
}
/**
* Tests rendering a path of more than four parts. The elider button should be
* visible and clicking it should 'open' and 'close' its drop-down menu.
......
......@@ -76,7 +76,7 @@ class LocationLine extends cr.EventTarget {
* @private
*/
updateNg_(components) {
this.components_ = components;
this.components_ = Array.from(components);
let breadcrumbs = document.querySelector('bread-crumb');
if (!breadcrumbs) {
......@@ -86,9 +86,9 @@ class LocationLine extends cr.EventTarget {
breadcrumbs.setSignalCallback(this.breadCrumbSignal_.bind(this));
}
let path = components[0].name;
let path = components[0].name.replace(/\//g, '%2F');
for (let i = 1; i < components.length; i++) {
path += '/' + components[i].name;
path += '/' + components[i].name.replace(/\//g, '%2F');
}
breadcrumbs.path = path;
......
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