Skip to content
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

Back button navigation regression in 7.5,7.x (fix #31238) #54270

Closed
Dosant opened this issue Jan 8, 2020 · 5 comments · Fixed by #54825
Closed

Back button navigation regression in 7.5,7.x (fix #31238) #54270

Dosant opened this issue Jan 8, 2020 · 5 comments · Fixed by #54825
Assignees
Labels
bug Fixes for quality problems that affect the customer experience regression Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.5.0

Comments

@Dosant
Copy link
Contributor

Dosant commented Jan 8, 2020

Kibana version: 7.5 , 7.x, (master is ok)

Describe the bug:

#31238 is reproduced in 7.5, 7.x

Any additional context:

Original fix was added in #32372 and landed into 7.0.0
The code which fixes the bug exists in master :

$location.url(absUrlHash).replace();

And also in 7.0 up to 7.2: https://github.com/elastic/kibana/blob/7.2/src/legacy/ui/public/chrome/api/sub_url_hooks.js#L45

But that piece of code is missing in 7.3, 7.4, 7.5, 7.x:

https://github.com/elastic/kibana/blob/d1f329619fb7236aad18fa34dda78b168be2bf06/src/legacy/ui/public/chrome/api/sub_url_hooks.js

I checked that in 7.5 the bug is reproduced (didn't check 7.3, 7.4)

Apparently the code is missing in 7.x because the original pr war reverted and then applied again:
#34300
#36226

But looks like on of those prs wasn't back-ported.

Also, was impossible to catch regression, because the original e2e test was skipped: #33468. Would be great to enable it back in scope of this fix.

@Dosant Dosant added bug Fixes for quality problems that affect the customer experience regression v7.5.0 labels Jan 8, 2020
@nreese nreese added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Jan 8, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet
Copy link
Contributor

It's hard to keep track of the fix history. @eliperelman @kobelb can we get more context on why this was reverted, then re-applied? Are we good to re-backport it to 7.x ?

@kobelb
Copy link
Contributor

kobelb commented Jan 9, 2020

The original PR was merged into master/7.2. After it merged, I brought up the following issues with it: #35991 and the decision was made to revert the original PR. This was reverted by #36226 which only merged to master, but was never backported. Spencer brought the lack of a backport to my attention and reverted our use of the fork in #46393 which was merged into master/7.5/7.4.1.

@joshdover
Copy link
Contributor

So just so I understand. The fork was reverted and backported in #46393 but the changes to src/legacy/ui/public/chrome/api/sub_url_hooks.js (#36226) was reverted from master, but not 7.x?

So all we need to is backport #36226 to 7.x/7.5 and reenable the e2e test (#33468).

@kobelb
Copy link
Contributor

kobelb commented Jan 14, 2020

So just so I understand. The fork was reverted and backported in #46393 but the changes to src/legacy/ui/public/chrome/api/sub_url_hooks.js (#36226) was reverted from master, but not 7.x?
So all we need to is backport #36226 to 7.x/7.5 and reenable the e2e test (#33468).

Correct... Until you brought it up, I didn't realize that Spencer's backport didn't revert everything from #34300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience regression Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants