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

Chrome doesn't remember viewmodel after history change #685

Open
GerardSmit opened this issue Apr 25, 2019 · 7 comments · May be fixed by #687
Open

Chrome doesn't remember viewmodel after history change #685

GerardSmit opened this issue Apr 25, 2019 · 7 comments · May be fixed by #687

Comments

@GerardSmit
Copy link

GerardSmit commented Apr 25, 2019 •
edited

When you leave the page by going back and then going forward, the viewmodel is reset to its initial state.

Example (GIF)

This is because Chrome doesn't remember the value of the <input type="hidden"> across history states. When we change the following in BodyResourceLinks.cs:

             // render the serialized viewmodel
             var serializedViewModel = ((DotvvmRequestContext) context).GetSerializedViewModel();
-            writer.AddAttribute("type", "hidden");
+            writer.AddAttribute("type", "text");
+            writer.AddAttribute("style", "display:none!important;");
             writer.AddAttribute("id", "__dot_viewmodel_root");
             writer.AddAttribute("value", serializedViewModel);
             writer.RenderSelfClosingTag("input");

then Chrome will remember the viewmodel:

Example (GIF)

When I searched for this issue, I found an issue from 2010 with the same problem (here) in WebForms. So I don't think this behavior won't change in Chrome if it didn't change in 9 years.

What would be the best solution here? Should we change the viewmodel input to an text-field and hide it (with maybe an setting in DotvvmConfiguration for compability)?

Browser: Google Chrome
Version: 73.0.3683.103 (Official Build) (64-bit)
Dotvvm build commit: dccddcf
Hosting: ASP.Net Core
@GerardSmit GerardSmit mentioned this issue Apr 25, 2019
Closed
@GerardSmit
Copy link
Author

I've created an PR (#686) with the change I made in the first message to fix the persisted viewmodel.

@exyi
Copy link
Member

exyi commented Apr 25, 2019

Hmm, that's strange, we actually introduced that hidden field back in 2015 for exactly this reason - #37. However, I can replicate this issue, it does not work in Chrome (while it works in FF). @tomasherceg You are using Chrome, aren't you? Do you know if it actually ever worked?

@tomasherceg
Copy link
Member

I am sure it worked in Chrome at the time we implemented this.
I wonder if there is any other input type that would persist the value - input[type=text] can be a problem for many scripts and libraries.

@GerardSmit
Copy link
Author

GerardSmit commented Apr 25, 2019 •
edited

I've done some research and on this page they suggest to use the History API.

The history API is available in most browser (https://caniuse.com/#search=history) with the exception of <IE9 and Opera Mini.

Maybe this is an good alternative for the input[type=text].

Edit: However the window.onpopstate is after the DOMContentLoaded event, which would break libraries that depends DotVVM to be loaded on window.onloaded.

This is a really complicated issue... 😕

@tomasherceg
Copy link
Member

I think we can use the History API. The only supported version of IE we support is 11.

Actually, I can imagine much more problems with changing the field to input[type=text] than with using window.onpopstate.
Using input[type=text] can break many selectors (including dozens of our UI tests which rely on the fact there is only one text field in the page). Also, I have seen a couple of libraries that search for all form fields and do something with them (animations, validations or focus handling). It can be tricky to convince the library to ignore this field.
I am not so worried about the fact that window.onpopstate comes after window.onload. Although it is unpleasant that DotVVM won't be able to detect whether the event onpopstate will be fired or not, so it has to load the default viewmodel which gets updated later, still - most of the scripts that rely on DotVVM are Knockout binding handlers which must be registered as early as possible and they expect that the viewmodel will be changed at any time, so it shouldn't be a big issue.
The only problem I see is that is can slow down the loading of the page as the default viewmodel will be prepared and then immediately updated with the last state. I think that we should keep the hidden field and apply the new state in onpopstate only when if it differs from the value in the hidden field.

@GerardSmit
Copy link
Author

Alright. I'll try to replace the input[type=hidden] with the History API this weekend. I'll close the PR #686 and make a new PR with the History API.

I also found out that there is a history.state property which returns the current state without having to wait for window.onpopstate 🎉.

@GerardSmit GerardSmit linked a pull request Apr 26, 2019 that will close this issue
Open
@GerardSmit
Copy link
Author

Done. I've created a new PR (#687) that uses the History API. Everything works fine in Edge, Chrome, Firefox and Internet Explorer. Since I don't have an Apple device at home I cannot test if the implementation works on Safari, maybe someone can test this?

Also I don't have checked if SPA mode works with the new implementation since this also works with the History API. It should work fine since I combine the old state and the new state, but it's better that someone can verify if this is still working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@GerardSmit @tomasherceg @exyi