V8: Repeatable textstring property grows with blank entries when open Rollback menu

[ReVoid]

Repeatable textstring datatype works not as expected when trying to navigate some other menus (detected a Rollback menu for now)

Umbraco version

Umbraco 8.6 (Pure Starter kit)

Reproduction

Bug summary

Steps to reproduce

1. Add property editor of "Repeatable textstring" type to any document type.
2. Focus the property UPD: or don't focus, it doesn't matter
3. Open a sidebar menu, "Info > Rollback" in my case
4. Return to the property, you will see it grown with a lot of empty entries

Expected result

Nothing changed with the property

Actual result

It has grown with lot of blank entries

[leekelleher]

I was curious to see if this happens on my Contentment package (specifically the Content Blocks editor), and it does! 😨

Looking for similarities between my editor and Repeatable Textstring, it's looking to be that when the editor uses $scope.model.value as it's ng-model, the Rollback controller code will manipulate it directly.

When it first sets the vm.currentVersion object, it does so by reference, not by value (e.g. it doesn't take a copy).
https://github.com/umbraco/Umbraco-CMS/blob/release-8.6.1/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/rollback/rollback.controller.js#L27

Then in createDiff, the property values are manipulated in various ways - depending on the value's object type ... serialized/deserialized, etc.
https://github.com/umbraco/Umbraco-CMS/blob/release-8.6.1/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/rollback/rollback.controller.js#L122-L134

That the point where the Repeatable Textstring (and other editors that use $scope.model.value) will be manipulated.

I haven't dug any deeper for a resolution - time is against me today - but IMO it'd be either cloning the vm.currentVersion or the individual property.value objects? Hopefully this may help someone if they are looking to fix this bug.

[kjac]

Whoa not good. This also affects NC.

Great detective work, @leekelleher ... I'm on it.

[kjac]

PR in #8163

[leekelleher]

Thanks @kjac! #h5yr 🚀

[PerplexDaniel]

Good work @kjac & @leekelleher, just FYI this issue has been reported on earlier (July 2019 😱) in #5791 and another PR was submitted by @bjarnef which also copies the property. So good you both arrived at the same solution 😄 !

Would be awesome if one of these 2 PRs can be merged in soon. Any editor that stores their value as JSON is messed up by this. Hopefully someone from core can merge this soon (@nul800sebastiaan perhaps ❤️ ?)

[nul800sebastiaan]

Thanks all, I've just closed the older issue #5791 merged the related PR, I'll close this as a duplicate! Fix is (finally) coming in 8.7.0.