-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[chrome/nav/lastUrl] do not track redirect routes #13432
Conversation
Is this failure un-related? |
Yeah, jenkins test this again please |
Sounds unrelated. If it passes this time, file an issue and I'll take a look to see what can be done. |
Appears to have failed with a different error now. Here are the previous failures: https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-selenium/6605 Jenkins, test it |
@stacey-gammon created issue #13455 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once the tests pass
Failed this time on:
|
1095569
to
41092b3
Compare
@stacey-gammon @tylersmalley looks like there was an actual bug; the kbnChrome directive was assuming that the ngRoute module was included, which is an opt-in feature for apps and not enabled in apps like the status page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, works nicely, thanks!
* [chrome/nav/lastUrl] do not track redirect routes * [chrome/subUrl] always track subUrl when no router (cherry picked from commit 9a34992)
* [chrome/nav/lastUrl] do not track redirect routes * [chrome/subUrl] always track subUrl when no router (cherry picked from commit 9a34992)
* [chrome/nav/lastUrl] do not track redirect routes * [chrome/subUrl] always track subUrl when no router (cherry picked from commit 9a34992)
Fixes #12933
Summary:
When you navigate to a URL that Kibana doesn't recognize it tries to be helpful and send you to discover. At the same time it tries to remember the URL you were last using in each app and bring back where you left off when you come back. Unfortunately, these two features recently collided. If you somehow ended up at an unknown URL that looked like the URL for an app other than discover Kibana would get confused and remember the bad URL and immediately redirect you to discover. If you didn't give up right away you would probably try to go back to the app, but since it Kibana is trying to be helpful it would send you right back to the bad URL and then back to discover... Stupid right?! Well, it won't happen anymore!
The chrome currently updates the
lastUrl
of each app whenever there is a navigation, even if the matched route is just redirecting users elsewhere. I can't think if a time that tracking the url of a redirect as thelastUrl
of an app would be desired, so this just filters out such navigation events.