Drag handler and mask are duplicated when reinitializing lost widget

[Piotrek Koszuliński]

1. Open placeholder sample.
2. Focus placeholder.
3. Click list button multiple times.
4. See that placeholder widget contains many drag handlers.

The reason is that when we reinitialize widget, we take the old DOM and repeat the initialization process, which creates drag handlers, masks and perhaps something else. During reinitialization we should search for those elements first and reuse them / recreate them if they already exist.

It may be related to #11112.

[Marek Lewandowski]

Pushed to ​t/11281 at dev and t/11281 at tests.

TC is within tt, since it depends on list plugin. I tried to reproduce it using purely contentDomInvalidated event, but with no luck.

Issue happens when CKEDITOR.plugins.widget.repository#checkWidgets() comes into a play.

From what i have seen, it's called on contentDomInvalidated event. It's attempting to create a widgets for new (unknown) widget wrappers (because in this instance widget was moved into list). As a result setupDragHandler is called with already inited wrapper code - creating yet another handler.

The same situation occurs with setupMask mask method, which is subject of issue #11112.

I added simplified solution, which in addition to cke_widget_new class adds cke_widget_reinit, which indicates that widget already has been inited at least once.

We may think about more sophisticated solution, storing all interface elements, and automatically creating or recovering them depending on situation.

[Piotrek Koszuliński]

I havent seen the patch yet, but it does not sound good. My initial idea is that if widget is reinitialised, if mask or drag handler already exist they should be reused. That's all.

Also, dt may be created for this issue. Before reinitialasing reset editable's innerHTML to break Dom references. This should be a test for checkwidgets, not contentdominvalidated.

[Piotrek Koszuliński]

I checked the code and in my opinion solution without class should be proposed as I described in comment:6. Then, we can have optimisation for new widgets and reinitialised widgets, but that requires a parameter and other modifications which are beyond this tc's scope.

[Marek Lewandowski]

Adjusted approach pushed here ​t/11281b. Tests for t/11281b are based on t/11281.

[Piotrek Koszuliński]

I rebased both branches and pushed one commit to t/11281b@dev.

R- because:

• when old container is reused event listeners are not added,
• as I mentioned in comment:6 a dt should be (because it may be) created for this, not tt - it works this way by design. It should be purely checkWidgets tests.

[Marek Lewandowski]

Listener was in fact not added. TC was renamed to test widgets.checkWidgets draghandler re usage.

Pushed changes to ​t/11281b at dev and tests.

[Marek Lewandowski]

Ticket status missclick, changed to review.

[Piotrek Koszuliński]

Drag handler reinitialization was fixed, but mask is still duplicated.

[Piotrek Koszuliński]

I added a test + the same fix for mask.

[Piotrek Koszuliński]

Fixed on master with ​git:eee34a6 on dev and 0a23f3f on tests.