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

replaceState does not update the URL in all scenarios when using PreserveOnRefresh #19813

Closed
jameskerrtaskize opened this issue Aug 21, 2024 · 2 comments · Fixed by #19817 or #19959
Closed
Assignees
Labels
BFP Bugfix priority, also known as Warranty bug Impact: Low Severity: Major

Comments

@jameskerrtaskize
Copy link

Description of the bug

I previously raised #19701 with a minimal reproducible example. That example was fixed but our actual use case requires a slightly more complicated reproducible example which I have attached to this ticket.

This issue is stopping us from being able to upgrade to 24.4

The example provided uses replaceState to update the URL. I think the issue also occurs when using forwarding. We switched to replaceState due to an earlier issue with PreserveOnRefresh which meant forwarding did not update the URL. Now neither mechanism works in this scenario.

launch the app with:

mvn jetty:run

Navigate to:

http://localhost:8080/parent/?blah=blah

You will find that the URL changes to:

http://localhost:8080/parent/child?blah=blah

but the desired outcome is that the URL changes to:

http://localhost:8080/parent/child?corrected=true

In ChildRoute.java you can delete the @PreserveOnRefresh and then repeat the test and it will work as desired.

Also, when @PreserveOnRefresh is enabled, you will see that you get two beforeEnter callbacks in ChildRoute but without it enabled, you get 1 as expected.

Expected behavior

replaceState should update the URL in all instances

Minimal reproducible example

preserveonrefresh-issue2.zip

Versions

  • Vaadin / Flow version: 24.4.x (tested on 24.4.9)
@jameskerrtaskize
Copy link
Author

reactEnable=false seems to fix the repro I attached

@mshabarov mshabarov added the BFP Bugfix priority, also known as Warranty label Aug 22, 2024
@tepi
Copy link
Contributor

tepi commented Aug 23, 2024

The issue was assigned to a developer and is currently being investigated

@tepi tepi self-assigned this Aug 27, 2024
@tepi tepi closed this as completed in b9d72b8 Aug 28, 2024
vaadin-bot added a commit that referenced this issue Aug 28, 2024
…#19817) (CP: 24.4) (#19844)

* Do not use callback if forward is called for PreserveOnRefresh target (#19817)

Fixes #19813

* Fix compilation error

---------

Co-authored-by: Teppo Kurki <teppo.kurki@vaadin.com>
caalador pushed a commit that referenced this issue Sep 18, 2024
Fixes #19794
Fixes #19822

Also fixes #19813 in a more coherent way. Relevant test is com.vaadin.flow.uitest.ui.PreserveOnRefreshForwardingIT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFP Bugfix priority, also known as Warranty bug Impact: Low Severity: Major
Projects
3 participants