Commit 3e988531 authored by tommycli's avatar tommycli Committed by Commit bot

Settings Router: Restrict navigateToPreviousRoute from going deeper.

Previous routes must now have equal or lesser depth.

BUG=640323
R=michaelpg@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2271003003
Cr-Commit-Position: refs/heads/master@{#414181}
parent 66a4eb70
...@@ -317,18 +317,19 @@ cr.define('settings', function() { ...@@ -317,18 +317,19 @@ cr.define('settings', function() {
}; };
/** /**
* Navigates to the previous route, but will never exit Settings. If there is * Navigates to the previous route if it has an equal or lesser depth.
* no previous route in the history, navigates to the immediate parent. * If there is no previous route in history meeting those requirements,
* this navigates to the immediate parent. This will never exit Settings.
*/ */
var navigateToPreviousRoute = function() { var navigateToPreviousRoute = function() {
var previousRoute = var previousRoute =
window.history.state && window.history.state &&
assert(getRouteForPath(/** @type {string} */ (window.history.state))); assert(getRouteForPath(/** @type {string} */ (window.history.state)));
if (previousRoute) if (previousRoute && previousRoute.depth <= currentRoute_.depth)
window.history.back(); window.history.back();
else else
navigateTo(settings.getCurrentRoute().parent || Route.BASIC); navigateTo(currentRoute_.parent || Route.BASIC);
}; };
window.addEventListener('popstate', function(event) { window.addEventListener('popstate', function(event) {
......
...@@ -54,4 +54,71 @@ suite('route', function() { ...@@ -54,4 +54,71 @@ suite('route', function() {
paths.add(route.path); paths.add(route.path);
}); });
}); });
/**
* Tests a specific navigation situation.
* @param {!settings.Route} previousRoute
* @param {!settings.Route} currentRoute
* @param {!settings.Route} expectedNavigatePreviousResult
* @return {!Promise}
*/
function testNavigateBackUsesHistory(previousRoute, currentRoute,
expectedNavigatePreviousResult) {
/**
* Returns a new promise that resolves after a window 'popstate' event.
* @return {!Promise}
*/
function whenPopState() {
return new Promise(function(resolve) {
window.addEventListener('popstate', function callback() {
window.removeEventListener('popstate', callback);
resolve();
});
});
}
settings.navigateTo(previousRoute);
settings.navigateTo(currentRoute);
settings.navigateToPreviousRoute();
return whenPopState().then(function() {
assertEquals(expectedNavigatePreviousResult,
settings.getCurrentRoute());
});
};
test('navigate back to parent previous route', function() {
return testNavigateBackUsesHistory(
settings.Route.BASIC,
settings.Route.PEOPLE,
settings.Route.BASIC);
});
test('navigate back to non-ancestor shallower route', function() {
return testNavigateBackUsesHistory(
settings.Route.ADVANCED,
settings.Route.PEOPLE,
settings.Route.ADVANCED);
});
test('navigate back to sibling route', function() {
return testNavigateBackUsesHistory(
settings.Route.APPEARANCE,
settings.Route.PEOPLE,
settings.Route.APPEARANCE);
});
test('navigate back to parent when previous route is deeper', function() {
settings.navigateTo(settings.Route.SYNC);
settings.navigateTo(settings.Route.PEOPLE);
settings.navigateToPreviousRoute();
assertEquals(settings.Route.BASIC, settings.getCurrentRoute());
});
test('navigate back to BASIC when going back from root pages', function() {
settings.navigateTo(settings.Route.PEOPLE);
settings.navigateTo(settings.Route.ADVANCED);
settings.navigateToPreviousRoute();
assertEquals(settings.Route.BASIC, settings.getCurrentRoute());
});
}); });
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