Umbraco-cms: JsDiff.diffWords in Rollback freezes browser

[LennardF1989]

After a deploy we somehow accidentally messed up our homepage. When trying to rollback to the last working version, the browser kept freezing up.

After diving into the code, I noticed that the point where the code started to hang was when it tried to call JsDiff.diffWords(property.value, oldProperty.value), obviously used to display the changes between the two version.

However, we have one very big Grid on this page, resulting in an oldProperty.value size of 2.3MB! JsDiff.diffWords cannot handle this size and will freeze up (especially considering the property.value at this point in time was near-empty, so the percentage of difference was insane).

Eventually, I managed to skip the diffing by replacing the function with a stub in the console: JsDiff.diffWords=function(a, b, c) { return undefined; }; and could successfully rollback the page.

I think there needs to be some sort of fail-safe in the whole diffing process. Either by truncating the data if it goes over a certain limit, or skipping it all together with a nice message why.

EDIT: Upon investigating, the Grid itself was not so big. It's all the stuff around it that virtually inflates the property value. The Grid itself contains like 10 sections and maybe 30 grid components in total. The actual property blows up in size because it's constantly repeating all the grid components in $allowedEditors (and we have a lot of them) and the full editor inside each placed grid component (and our grid components have a lot of configuration).

So besides JsDiff.diffWords needing a fail-safe, the value-format stored by the Grid datatype is HIGHLY inefficient, so that might be an issue all on its own.

[bjarnef]

Not sure if specific JsDiff.diffWords has issues with handling large data size, but since rollback compare each part which is different it might repeat a lot of entries and I know if using ng-repeat on 1000 element, maybe also fewer, it has a many DOM elements and the browser freeze or become slow. Adding track by can improve the permance and use one-way binding ud possible, but not sure if it fix the problem completely.
https://github.com/umbraco/Umbraco-CMS/issues/5755

[LennardF1989]

I'm fairly certain it's the JsDiff.diffWords as I set a breakpoint right before it executed the large property-value and performed it manually. It will immediately lock up the browser-tab and inspector.

[nul800sebastiaan]

We are aware that the stored data for grid element was an unfortunate choice and unfortunately can't be changed since we need to maintain backwards compatibility. With all of the experience we have had with Grid over the years, we've started the process of building a better block editor https://github.com/umbraco/rfcs/pull/12.

As for the performance issue, that's pretty unfortunate! What do you suggest to do about it, given that we might have this amount of data?

[LennardF1989]

@nul800sebastiaan I knew about the other one and am not blaming anyone for it :) Just added it for some context as to why JsDiff.diffWords hung up.

I see 3 possible solutions:

• Skip diffing big property values
• Truncate big property values before diffing
• Cap the amount of diffs that can be detected in JsDiff.diffWords

Skipping is relatively easy, but just like truncating, you will need to find a sweetspot... I've went from 64000 length to 10000 length in my tests, and in neither case would the diffing ever complete without crashing the browser-tab. So diffing a piece of Json has completely different results compared to diffing a piece of text. On the official documentation I'm seeing JSDiff also has a diffJson funtion which might perform a lot better in this case (https://github.com/kpdecker/jsdiff). So that's worth a shot.

I do feel option 3 is the most reasonable one, but I'm not seeing hooks for that in the library...

We could set a boolean on the resulting object that a particular property was treated differently so it can be shown as such on the UI for the end-user.

All in all, I guess this is a case of just trial-and-error to figure out the best solution.

[croban]

Is there some quick fix for this issue, because rollback does not work in v8.4
/umbraco/backoffice/UmbracoApi/Content/GetRollbackVersion?versionId=879&culture= request never finish inside ajax Reqest but if I open it directly inside browser response is there in milliseconds.

[Attackmonkey]

+1 on this issue, we've hit it on a site this morning. Using the console hack outlined in the report allowed us to continue with the rollback without hanging the browser. Is there a way we could have it diff the simple property types and then have a link to do a diff on the grids as a separate task (although that doesn't get round the issue of size, it just breaks it into chunks)?

[enkelmedia]

+1 here as well. I would be quite OK with a setting to disable the diff visualizer? Maybe that's a way forward, or maybe disable it for certain property editors?

[emmaburstow]

Looking at the thread - it would be fab is we could find a solution that used option 3 of the above suggestions 'Cap the amount of diffs that can be detected in JsDiff.diffWords'. There needs to be some investigation of whether the library has hooks for it. Otherwise, this would be a good issue to pick up for a pr for anyone who has time to investigate the 3 suggestions.

Em

[umbrabot]

Hi @LennardF1989,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

[nul800sebastiaan]

Using diffJson seems to take us a long way, it immediately alleviates all the stuff we store as JSON. I've made a PR here to evaluate, please try it out and let us know how it goes! https://github.com/umbraco/Umbraco-CMS/pull/9381

[lars-erik]

9381 makes a diff take 20 seconds where it previously froze the browser for 30+ min.

I say merge! 🎉❤

(should say send instead of request, but it's the highest!)

[nul800sebastiaan]

Fixed in https://github.com/umbraco/Umbraco-CMS/pull/9381