Skip to content

Commit

Permalink
Back button navigation fix 31238 (elastic#32372)
Browse files Browse the repository at this point in the history
* initial fix for navigation issue + Spencer
* added navigation test for issue elastic#31238 
* added more validations to the navigate back test
  • Loading branch information
lizozom authored and Liza Katz committed Mar 5, 2019
1 parent 4e89a69 commit 9237f49
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/legacy/ui/public/chrome/directives/kbn_chrome.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import React from 'react';
import ReactDOM from 'react-dom';
import $ from 'jquery';
import url from 'url';

import { uiModules } from '../../modules';
import {
Expand Down Expand Up @@ -57,7 +58,7 @@ export function kbnChromeProvider(chrome, internals) {
},

controllerAs: 'chrome',
controller($scope, $rootScope, Private) {
controller($scope, $rootScope, Private, $location) {
const getUnhashableStates = Private(getUnhashableStatesProvider);
const subUrlRouteFilter = Private(SubUrlRouteFilterProvider);

Expand All @@ -73,6 +74,19 @@ export function kbnChromeProvider(chrome, internals) {
}
}

$rootScope.$on('$locationChangeStart', (e, newUrl) => {
// This handler fixes issue #31238 where browser back navigation
// fails due to angular 1.6 parsing url encoded params wrong.
const absUrlHash = url.parse($location.absUrl()).hash;
const decodedAbsUrlHash = decodeURIComponent(absUrlHash);
const hash = url.parse(newUrl).hash;
const decodedHash = decodeURIComponent(hash);
if (absUrlHash !== hash && decodedHash === decodedAbsUrlHash) {
// replace the urlencoded hash with the version that angular sees.
$location.url(absUrlHash).replace();
}

});
$rootScope.$on('$routeChangeSuccess', onRouteChange);
$rootScope.$on('$routeUpdate', onRouteChange);
updateSubUrls(); // initialize sub urls
Expand Down
74 changes: 74 additions & 0 deletions test/functional/apps/home/_navigation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import expect from 'expect.js';


export default function ({ getService, getPageObjects }) {
const browser = getService('browser');
const PageObjects = getPageObjects(['dashboard', 'common', 'home', 'timePicker']);
const appsMenu = getService('appsMenu');
const kibanaServer = getService('kibanaServer');
const fromTime = '2015-09-19 06:31:44.000';
const toTime = '2015-09-23 18:31:44.000';

describe('Kibana browser back navigation should work', function describeIndexTests() {

before(async () => {
await PageObjects.dashboard.initTests();
await kibanaServer.uiSettings.disableToastAutohide();
await browser.refresh();
});

it('detect navigate back issues', async ()=> {
let currUrl;
// Detects bug described in issue #31238 - where back navigation would get stuck to URL encoding handling in Angular.
// Navigate to home app
await PageObjects.common.navigateToApp('home');
const homeUrl = await browser.getCurrentUrl();

// Navigate to discover app
await appsMenu.clickLink('Discover');
const discoverUrl = await browser.getCurrentUrl();
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
const modifiedTimeDiscoverUrl = await browser.getCurrentUrl();

// Navigate to dashboard app
await appsMenu.clickLink('Dashboard');

// Navigating back to discover
await browser.goBack();
currUrl = await browser.getCurrentUrl();
expect(currUrl).to.be(modifiedTimeDiscoverUrl);

// Navigating back from time settings
await browser.goBack(); // undo time settings
await browser.goBack(); // undo automatically set config, should it be in the history stack? (separate issue!)
currUrl = await browser.getCurrentUrl();
// Discover view also keeps adds some default arguments into the _a URL parameter, so we can only check that the url starts the same.
expect(currUrl.startsWith(discoverUrl)).to.be(true);

// Navigate back home
await browser.goBack();
currUrl = await browser.getCurrentUrl();
expect(currUrl).to.be(homeUrl);
});
});

}
1 change: 1 addition & 0 deletions test/functional/apps/home/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export default function ({ getService, loadTestFile }) {
return browser.setWindowSize(1200, 800);
});

loadTestFile(require.resolve('./_navigation'));
loadTestFile(require.resolve('./_home'));
loadTestFile(require.resolve('./_add_data'));
loadTestFile(require.resolve('./_sample_data'));
Expand Down

0 comments on commit 9237f49

Please sign in to comment.