BeforeLeaveEvent::postpone() method doesn't prevent to update the browser URL

[denis-anisimov]

Let's say you have a view with a router link.
The view is BeforeLeaveObserver and has a method beforeLeave which postpones the navigation.

The issue is that the browser URL is changed even though the navigation is postponed.

@Route("help")
public class HelpView extends VerticalLayout implements 
        BeforeLeaveObserver {

    public HelpView() {
        add(new RouterLink("To main view", MainView.class));
    }

    @Override
    public void beforeLeave(BeforeLeaveEvent beforeLeaveEvent) {
            beforeLeaveEvent.postpone();
    }

I'm not sure whether it should be fixed or not.
It's confusing since the navigation has not happened.
But on the other hand there is no way to cancel the navigation. It's just postponed for a while and will happen anyway once the proceed method is called.
So may be it's acceptable to have the url changed even though the navigation has not happened yet.

The reason of the issue is : router link is handled on the client side first in a generic way.
The browser location is changed on the client side via the com.vaadin.client.flow.RouterLinkHandler which delegates the call to the ScrollPositionHandler.beforeNavigation method.
This method pushes the new location to the browser history immediately.

To be able to fix this particular issue we will need to ask the server-side before do anything on the client side and update the history after the confirmation.
It's quite complicated and is needed only for the postpone specific case but the RouterLink is a generic functionality which doesn't depend anyhow on the postpone.
Also I'm not sure whether the browser allows to do roundtrip on click at all.

Anyway, I don't think it's critical issue and not sure whether it needs to be fixed at all.

[denis-anisimov]

The original issue is #3613

[pleku]

Criticalness aside, it still an issue that can leave an undesired navigation states visible for the end user.

Since the navigation will already cause a new automatic history state to be added, we will anyway have another state entry there. It might just be that this state is actually never loaded in the UI but only in the history state, meaning back/forward navigation can cause confusion and maybe even errors if application developer is not handling this.

Maybe the postponed state should actually something that the user can navigate back from? Eg:

• A form open, state: my-form
• click link to navigate to my-profile will cause a postponed navigation showing a confirmation dialog "are you sure?"
  ->
  a) Should there be a new state eg. my-form/confirm or my-form/postponed such ?
  b Should we push the exact same previous state again visible with replaceState, make it my-form again, and then do a replace state after the navigation has been continued with the desired state (here my-profile) ?

b) will cause there to be the original state twice in the history, if the user never does the continuation of the navigation after postpone.
a) is something that the application developer can already do know, but it is another question should we enforce it or make it more easy to achieve this. We could make it automatic, but then we would have to make it a magic url so that it doesn't mess up the normal navigation, eg. Flow should be able to ignore any navigation to the "postponed" state and instead open the base view for that.

After briefly looking, there is no way for deleting history state entries, but you can hack around that. Don't want to take that route.

Please, @Legioth and @jouni any thoughts ?

[jouni]

Conceptually this is the same issue as closing a dialog: https://github.com/vaadin/vaadin-dialog/issues/56

Am I right in assuming a history state change has to happen in order for Flow to know and notify BeforeLeaveObservers? If not, then this should be solvable with a simple callback. But since that’s an obvious solution, I assume it’s not possible like that.

Can it work like this:

• Onmy-form/
• User navigates to profile view (URL changes to my-profile/)
  • If there are no changes in the form, Flow shows the profile view (URL stays the same)
  • If there are unsaved changes in the form, Flow shows the confirmation dialog UI instead of the profile view (URL is still my-profile/)
    • User pressed “Cancel”, Flow navigates back (URL reverted to my-form/)
    • User presses “Proceed”, Flow shows the profile view UI (URL is still my-profile/)

What would be the problems in that approach?

[pleku]

The challenge is that currently there is a state entry created on client side before the server side round trip, thus when the postpone decision is made on the server side, there has been an additional state already pushed (with the new URL) and we cannot remove that, and replacing it with replaceState would cause there to be two states with the same URL and that will cause confusion with back/forward navigation.

After looking at the code, it seems that we are currently blocking the browser from pushing a state entry, and instead pushing it ourselves before the change is sent to server. This is done in ScrollPositionHandler.

For making it work as Jouni described, we would need to wait for pushing the new state until the server has responded that whether to push the state or not (not when postponed).

I'm not sure how much this might break other things. But it looks to me we should investigate and preferably fix this to work so that:

• The history state is not updated eagerly before the request, but after the response is received.
• Any postpone action would prevent the history from being updated
• The user can still do a push- or replaceState if they desire, but they have to bear the consequences

[Legioth]

Preferably, the old URL should still be retained until the user has confirmed that they want to go to the new URL. Otherwise, things will be slightly weird when hitting refresh in the browser. Things would still be slightly weird since there's no preserveOnRefresh which means that unsaved changes will still be lost and the dialog will no longer be open, but the user is at least still on the same URL.

One potential ugly way of achieving this is to do history.back() to pop the unwanted new state and then also set things up to ignore the inevitable historychange event.

[pleku]

It should be possible to just delegate the push state handling to the server side when user has clicked a router link. The server side needs to
a) Handle the case of postponing the navigation and push or not push the state later
b) Understand that there is some state that must be pushed with the navigation because the client side scroll position capturing needs that
c) preferably log info or warning when user uses the History.pushState with the Router since any such action might break user navigation or scroll restoration. This happens already now, but there is no way of detecting that.

Need to investigate this. I think also it would be preferable to push the history state on the client side earlier than via execute javascript. Not sure if this is necessary though.

[Legioth]

router-link navigation could be handled by not doing anything at all until after a round trip, but that is not the case for popstate events.

It would also be desirable that the URL in the browser would be updated immediately when clicking a router-link instead of only after a round trip, but that case is not as important as the other issues discussed in this ticket.

[samulivaadin]

We have a use case that is very similar to this, but with exception that the navigation is done programmatically at server side (UI.navigate). Its a bit confusing that navigation of the view can be prevented but despite that the url updates.

[Wnt]

Currently if I want to use BeforeLeaveObserver's BeforeLeaveEvent.postpone() I also need to make the my view implement AfterNavigationObserver to be able to store the Location the browser has when coming into the view. I need this location in the beforeLeave method to call

UI.getCurrent().getPage().executeJavaScript("history.replaceState({},'','" + originalLocation + "');");

whenever I call BeforeLeaveEvent.postpone(), otherwise if the user cancels the exit action, the browser will continue to show the canceled navigation action destination location in the address bar.

Also if the user does decide to continue with the exit action and I call ContinueNavigationAction.proceed() I need to have the destination location available so that I can also call

UI.getCurrent().getPage().executeJavaScript("history.replaceState({},'title','" + destination + "');")

I think both should happen automatically when I call BeforeLeaveEvent.postpone() or ContinueNavigationAction.proceed().

Here's the whole logic:

@Override
public void afterNavigation(AfterNavigationEvent event) {
	// store the current location so that we can restore that in beforeLeave if needed
	originalLocation = event.getLocation().getPathWithQueryParameters();
}

@Override
public void beforeLeave(BeforeLeaveEvent event) {
	// if user has unsaved changes
	if (askBeforeLeave) {
		// postpone the view change
		ContinueNavigationAction postponedNavigationAction = event.postpone();
		// and also replace the top most history state in the browser with this view's location
		// https://github.com/vaadin/flow/issues/3619
		UI.getCurrent().getPage().executeJavaScript("history.replaceState({},'','" + originalLocation + "');");
		
		// show the user a dialog where they can continue or cancel the exit action
		Dialog dialog = new Dialog();
		Button cancelButton = new Button("Cancel", VaadinIcon.CLOSE.create(), cancel -> {
			dialog.close();
		});
		Button confirmButton = new Button("Confirm", VaadinIcon.CHECK.create(), confirm -> {
			dialog.close();
			
			// change to the view where the user was trying to navigate to
			postponedNavigationAction.proceed();
			// and also update the address bar to reflect that location 
			String destination = event.getLocation().getPathWithQueryParameters();
			UI.getCurrent().getPage().executeJavaScript("history.replaceState({},'','" + destination + "');");
		});
		dialog.add(new H1("Are you sure?"), new Paragraph("Discard unsaved changes and leave this view?"),
				cancelButton, confirmButton);
		dialog.open();
	}
}

[pleku]

So to recap there are three different things to fix here

1. clicking a routerlink updates the URL too eagerly. It should be done after the roundtrip. This should be possible to fix but requires some changes.
2. doing UI.navigate and then postpone() should not update the URL but only after the navigation is continued. This should be a straight forward fix to make.
3. when the user does back/forward navigation and causes a popstatevent which is sent to server side, the URL has changed before we can affect it. The event cannot be canceled for obvious reasons. This is problematic and for the postpone case it is unknown if we can fix this. Maybe we could try to push back the old state with history.pushState() (?) if the user has triggered the postpone action from server side, but this needs to be investigated.

In cases 1 & 3 also the scroll position needs to be considered as it needs to be retained.
For this ticket, I would focus now on fixing 1 & 2 and briefly investigate if 3 is possible to fix. If there are no ideas, another issue can be opened for further investigation.

[pleku]

Shipped in 2.2.0 and to be released in 2.1.10. Still being ported to 3.x versions