Fixed the persisted viewmodel in Chrome

[GerardSmit]

I've replaced the <input type="__dot_viewmodel_root"> with the history API as suggested in #685 (comment). I've tested and verified that the new implementation works 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.

Since the input tag is not necessary anymore I've also added the configuration option 'InlineViewModel', which puts the viewmodel in the script-tag instead of the input-tag. I made this an option so libraries that rely on __dot_viewmodel_root don't break.

Before (Debug = false, InlineViewModel = false)

After (Debug = false, InlineViewModel = true)

If this is out-of-scope for this PR I can revert this change and make this a new PR.

---

fixes #685

[GerardSmit]

It was working a little bit too well. I've noticed that when you reload the page the viewmodel was also being restored. This has been fixed by checking on load if the page was reloaded through the Performance API Level 1 and Performance API Level 2.

[exyi]

Thank you for pushing this forward!

Since the input tag is not necessary anymore I've also added the configuration option 'InlineViewModel', which puts the viewmodel in the script-tag instead of the input-tag. I made this an option so libraries that rely on __dot_viewmodel_root don't break.

This one is a bit problematic, I think we should handle it separately. I see two problems:

• As is, your PR introduces a XSS vulnerability as we don't encode characters like <, >, ... in the JSON. Putting </script> into a string property would end the script block and giving you "freedom" access to do anything in the HTML.
• Although it may be a bit shorter, parsing JSON is faster that parsing generic Javascript, so this optimization may not be worth it. Comparing raw sized of HTML is also not very fair, as almost everything is gzip-compressed on the network.

It was working a little bit too well. I've noticed that when you reload the page the viewmodel was also being restored. This has been fixed by checking on load if the page was reloaded through the Performance API Level 1 and Performance API Level 2.

Hmm, this is how it works correctly in Firefox - Firefox does preserve form fields on reload, and I use it fairly often to reload a page while writing some comment. Ctrl+F5 drops the state. I think we should not break this behavior, but I don't have any idea how to do so :(

[GerardSmit]

As is, your PR introduces a XSS vulnerability as we don't encode characters like <, >, ... in the JSON. Putting </script> into a string property would end the script block and giving you "freedom" access to do anything in the HTML.

Whoops, I didn't realize that the old way used WriteUnencodedText:

dotvvm/src/Redwood.Framework/Controls/Infrastructure/BodyResourceLinks.cs

Lines 31 to 37 in 49c40ed

	var serializedViewModel = context.RequestContext.GetSerializedViewModel();
	writer.RenderBeginTag("script");
	writer.WriteUnencodedText("redwood.viewModels.");
	writer.WriteUnencodedText(context.CurrentPageArea);
	writer.WriteUnencodedText("=");
	writer.WriteUnencodedText(serializedViewModel);
	writer.WriteUnencodedText(";\r\n");

I'll revert this change. Good catch.

Although it may be a bit shorter, parsing JSON is faster that parsing generic Javascript, so this optimization may not be worth it. Comparing raw sized of HTML is also not very fair, as almost everything is gzip-compressed on the network.

At work we're still using WebForms that has on some pages a giant viewstate, which our SEO team isn't a big fan of. That's why I made this change. But nonetheless I'll revert this change instead of fixing it. An XSS exploit is much more troublesome.

Hmm, this is how it works correctly in Firefox - Firefox does preserve form fields on reload, and I use it fairly often to reload a page while writing some comment. Ctrl+F5 drops the state. I think we should not break this behavior, but I don't have any idea how to do so :(

I couldn't find a solution for this either. I reverted the <input type="hidden"> back and made it so only Chrome and Edge (Edge will be Chromium in the future and doesn't save the viewmodel either) will use the History API.

[GerardSmit]

For some reason the TypeScript compiler moved the variables around...

I'll try to update my TypeScript compiler since the package is not defined in package.json.
Wait, it is... Now I'm confused 😅

[tomasherceg]

What about this:

1. Keep the input[type=hidden]
2. When leaving the page, store the value using pushState and also in the hidden field.
3. When the window.onpopstate is fired and it's not a page refresh, we should check whether the state in History API is different from the hidden field value. If it is different, apply it to the viewmodel; otherwise, do nothing.

This should keep the behavior of non-Chrome browsers unchanged, and fix the Chrome behavior.

[GerardSmit]

1. Keep the input[type=hidden]
2. When leaving the page, store the value using pushState and also in the hidden field.

This is already the case in the commit I made yesterday.

3. When the window.onpopstate is fired and it's not a page refresh, we should check whether the state in History API is different from the hidden field value. If it is different, apply it to the viewmodel; otherwise, do nothing.

This is the problem. There is no way to detect if the reload was a hard-reload. Chromium browsers reset their input-fields when the page was reloaded, Firefox does this only when it was a hard-reload. @exyi said he uses this behavior and this is why I only use the History API on Chromium-browsers.

[tomasherceg]

Oh, I see - that should work. :-)

I am not sure about the detection of Chrome browser - since Microsoft will use Chromium in the next version of Edge, I expect there will be the same problem with the hidden fields.
I think that we don't need to check if the browser is Chrome specifically. We should check if the value of the hidden field got lost. What about adding a second hidden field that would be empty by default, and we set some value to it when we are leaving the page. Then we'd be able to detect if we need to load the state or not.

[GerardSmit]

I am not sure about the detection of Chrome browser - since Microsoft will use Chromium in the next version of Edge, I expect there will be the same problem with the hidden fields.

The development build of Edge (that is based on Chromium) has the same user agent and vendor (Google Inc.) as Chrome. I'm not sure if this is going to change.

I think that we don't need to check if the browser is Chrome specifically. We should check if the value of the hidden field got lost. What about adding a second hidden field that would be empty by default, and we set some value to it when we are leaving the page. Then we'd be able to detect if we need to load the state or not.

I think this is a better solution. This way we don't have to whitelist browsers that doens't support reloading the input[type=hidden] and if Chromium changes their behavior in the future we don't have to change anything.

I'll update this PR in the evening.

[GerardSmit]

Done. I've added a new hidden input called __dot_persisted_viewmodel_root. Whenever this input has an value the value will be used as viewmodel. If the current request is not a reload and the viewmodel is set in the History API, the History API will be used as viewmodel. And when both are not set, the default __dot_viewmodel_root will be used.

[GerardSmit]

After more testing I've found a bug with the History API. When you click a link that navigates to the same URL, Chrome doesn't add a new item in the history (page back). Because of this it'll use the same state thus the old viewmodel is being loaded, while you might expect the initial viewmodel.

I've the feeling that this is a never ending war with Chrome 😅.

[tomasherceg]

What if we try to change input[type=hidden] to input[type=text] in onbeforeunload? Maybe Chrome will remember the value.