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.

Sign up for GitHub

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

Jump to bottom

Browser history navigation fails silently in v23 with v14 bootstrapping enabled #14323

Closed
SoylentBob opened this issue Aug 11, 2022 · 5 comments · Fixed by #14742
Closed

Browser history navigation fails silently in v23 with v14 bootstrapping enabled #14323

SoylentBob opened this issue Aug 11, 2022 · 5 comments · Fixed by #14742
Assignees
@mcollovati
Labels
bug Impact: Low navigation prerelease version for Vaadin 14.9.0 Released with Vaadin 14.9.0.beta1 Released with Vaadin 23.1.13 Released with Vaadin 23.2.6 Severity: Major
Projects
OLD Vaadin Flow bugs & maintenance (V... OLD Vaadin Flow ongoing work (Vaadin ...

Comments

@SoylentBob
Copy link

Description of the bug

When using server side navigation (e.g. UI.getCurrent().navigate("some-link")) in a Vaadin v23.1.6 Application built with gradle and v14 bootstrapping enabled, the browser navigation history is displayed correctly, but going back via the browsers history does not open the previously opened pages, even though the URL in the browser is updated correctly.

Other links such as HTML Anchors or Router Links seem to not be affected by this issue.

Expected behavior

Going through the browsers history should work even with server-side navigation and v14 bootstrapping enabled.

Minimal reproducible example

Clone https://github.com/SoylentBob/vaadin-navigation-bug (the project is based on https://github.com/vaadin/base-starter-gradle)

  1. Start the application with the command ./gradlew jettyRun
  2. Open http://localhost:8080 in your browser.

Result of Step 2

  1. Click on the "Open second view", you should be on the URL http://localhost:8080/second now

Result of Step 3

  1. Click on the browsers back button, your browsers URL should have changed to http://localhost:8080, without actually displaying the "Main" view.

Result of Step 4

Setting the useDeprecatedV14Bootstrapping flag in the build.gradle file to false seems to fix the bug in the attached test project, but is not feasible at the moment for the project that we noticed the bug in.

Versions

  • Vaadin / Flow version: 23.1.6 (v14 Bootstrapping enabled)
  • Java version: 17
  • OS version:
  • Browser version (if applicable):
  • Application Server (if applicable):
  • IDE (if applicable):
@caalador caalador moved this from Needs triage to New P2 in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Aug 11, 2022
@mshabarov
Copy link
Contributor

Might be related to vaadin/multiplatform-runtime#115 (comment)

@mshabarov mshabarov moved this from New P2 to P1 - High priority in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Aug 17, 2022
@mshabarov mshabarov mentioned this issue Aug 17, 2022
Closed
@mcollovati
Copy link
Collaborator

When performing a server-side navigation, the client receives a JavaScript history.pushState instruction to update the history

setTimeout(() => window.history.pushState($0, '', $1))

On flow client, in PopStateHandler we register a ResponseHandlingEndedHandler to catch the current window.location path and query string.
In addition, a popstate listener is registered, to react to browser back and forward navigation buttons (events are not fired for history.pushState). This listener compares stored location information with the current value; if they are different, the server is notified of the change; otherwise there is no server roundtrip.

The issue here seems to be that the ResponseHandlingEndedHandler is running before the execution of history.pushState (probably because of setTimeout), so when the back button is pressed and the popstate listener is executed, the current window.location is the same as the one we stored in ResponseHandlingEndedHandler

mcollovati added a commit that referenced this issue Oct 5, 2022
@mcollovati
fix: defer location and query tracking
5cca17a 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
@mcollovati mcollovati mentioned this issue Oct 5, 2022
Merged
9 tasks
@mcollovati mcollovati self-assigned this Oct 5, 2022
@mcollovati mcollovati moved this from Ready To Go to In progress in OLD Vaadin Flow ongoing work (Vaadin 10+) Oct 5, 2022
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from In progress to Done - pending release Oct 10, 2022
caalador pushed a commit that referenced this issue Oct 10, 2022
@mcollovati
fix: defer location and query tracking (#14742)
3ae1b30 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from P1 - High priority to Closed Oct 10, 2022
vaadin-bot pushed a commit that referenced this issue Oct 10, 2022
@mcollovati @vaadin-bot
fix: defer location and query tracking (#14742)
87fe526 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
vaadin-bot pushed a commit that referenced this issue Oct 10, 2022
@mcollovati @vaadin-bot
fix: defer location and query tracking (#14742)
8d910a0 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
vaadin-bot pushed a commit that referenced this issue Oct 10, 2022
@mcollovati @vaadin-bot
fix: defer location and query tracking (#14742)
dbffe96 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
mcollovati added a commit that referenced this issue Oct 10, 2022
@mcollovati
fix: defer location and query tracking (#14742)
ac56730 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
@mcollovati mcollovati mentioned this issue Oct 10, 2022
Merged
caalador pushed a commit that referenced this issue Oct 10, 2022
@mcollovati
fix: defer location and query tracking (#14742) (#14793)
c37d122 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
vaadin-bot pushed a commit that referenced this issue Oct 10, 2022
@mcollovati @vaadin-bot
fix: defer location and query tracking (#14742) (#14793)
c969959 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323
caalador pushed a commit that referenced this issue Oct 11, 2022
@vaadin-bot @mcollovati
fix: defer location and query tracking (#14742) (#14793) (#14796)
c12bb25 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323

Co-authored-by: Marco Collovati <marco@vaadin.com>
caalador pushed a commit that referenced this issue Oct 11, 2022
@vaadin-bot @mcollovati
fix: defer location and query tracking (#14742) (#14792)
dd1a155 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323

Co-authored-by: Marco Collovati <marco@vaadin.com>
caalador pushed a commit that referenced this issue Oct 11, 2022
@vaadin-bot @mcollovati
fix: defer location and query tracking (#14742) (#14791)
97b7e71 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323

Co-authored-by: Marco Collovati <marco@vaadin.com>
caalador pushed a commit that referenced this issue Oct 11, 2022
@vaadin-bot @mcollovati
fix: defer location and query tracking (#14742) (#14790)
69ee918 
Track of location and query string after response from server
happens too early when handling server side navigation,
because history.pushState is executed within a setTimeout.
This change defers the tracking to the next event loop cycle
in order to get the correct values.

Fixes #14323

Co-authored-by: Marco Collovati <marco@vaadin.com>
SoylentBob added a commit to SoylentBob/vaadin-navigation-bug that referenced this issue Oct 24, 2022
@SoylentBob
Upgrade to Vaadin 23.1.10 to check if the bug still exists
8643730 
Upgraded the project to Vaadin 23.1.10 to check whether the bug described in vaadin/flow#14323 still exists.
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.1.13.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.2.6.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 14.9.0.beta1 and is also targeting the upcoming stable 14.9.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mcollovati mcollovati

Labels
bug Impact: Low navigation prerelease version for Vaadin 14.9.0 Released with Vaadin 14.9.0.beta1 Released with Vaadin 23.1.13 Released with Vaadin 23.2.6 Severity: Major
Projects
OLD Vaadin Flow bugs & maintenance (V...
Closed
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: defer location and query tracking vaadin/flow
5 participants
@caalador @mcollovati @SoylentBob @vaadin-bot @mshabarov