Fix undo related issues

[dmolineus]

Haste replaces the default undo behaviour of the DC_Table if any haste_data exist in the undo record. This pull request solves two issues:

1. As it replaces the default behaviour it has to be executed even if no haste related hooks are registered. Imagine an undo record containing data of an removed extension. The hasData() check is superfluous here, because it's already checked before replacing the default behaviour.

2. The onundo_callback was very likely not triggered because the related data container wasn't loaded.

I wonder why haste replaces the default behaviour and not just using the onundo_callback to execute it's extended features. This might be an additional change. I stick the current implementation here.

[qzminski]

The reason Haste replaces the default button callback is that the onundo_callback does not provide any information on which tl_undo record is being restored and thus we don't know the haste_data to restore.

As it replaces the default behaviour it has to be executed even if no haste related hooks are registered. Imagine an undo record containing data of an removed extension. The hasData() check is superfluous here, because it's already checked before replacing the default behaviour.

The hasData() is used here again because the method is static and can be called by third party software – highly improbable, but still. Thus please keep that condition, just move it before line 112: https://github.com/codefog/contao-haste/blob/08372e5a114d1c7826548f0fb0eb4fd0e913b4a0/library/Haste/Util/Undo.php#L112

The onundo_callback was very likely not triggered because the related data container wasn't loaded.

That's a valid point I think :+1:

[dmolineus]

The reason Haste replaces the default button callback is that the onundo_callback does not provide any information on which tl_undo record is being restored and thus we don't know the haste_data to restore.

It does provide the instance of the current DC_Table which contains the id of the undo record. But nevermind, I just wondered. :-)

The hasData() is used here again because the method is static and can be called by third party software – highly improbable, but still. Thus please keep that condition, just move it before line 112

Added with d87a2b7.

[qzminski]

It does provide the instance of the current DC_Table which contains the id of the undo record. But nevermind, I just wondered. :-)

So you mean that the $dc->id is in fact a tl_undo.id? Then we could actually refactor this code… but maybe another time during some complete rewrite 😉 For now this merge should do the job.

[qzminski]

Released in 4.24.13, thank you!