Ckeditor5: There should be some way exclude DOM events fired by certain UIElements from default handlers

Created on 4 Apr 2019  ·  13Comments  ·  Source: ckeditor/ckeditor5

Problem

ATM the widget takes over the DOM events like mousedown (also keydown, delete) and processes them as it thinks fit.

I realized when implementing https://github.com/ckeditor/ckeditor5/issues/1509 that this behaviour causes some issues. For instance, because mousedown is handled by the widget, I wasn't able to create any dynamic form elements (<select>) using React inside a UIElement in the widget. They render, that's alright, but you can't click them, focus etc. because widget wants to handle everything.

What I thought was a solution

Unfortunately the problem is not as simple as "remove custom [event] handling when even target is UIElement or a descendant" because by doing this, we'd prevent widget selection when an UIElement is clicked, for instance a media widget uses UIElement to display the YouTube preview, and we definitely want clicking it to focus the widget.

I also tried to attach a native DOM mousedown listener on the <select> I rendered, and stop propagation of the event before it reaches the editable, before CKEditor's widget logick kicks in. I would make sense except that... React uses a single global listener for each event and it heavily depends on the fact that the event does bubble towards the root of the application. Stopping it like that simply breaks React so it's a dead–end.

An actual solution

What I guess we need is some API to tell the widget that a particual HTMLElement should be excluded from custom event handling.

How do we do that?

cc @jodator @Reinmar @mlewand

dx intro widget dx 2 feature

Most helpful comment

🎉

All 13 comments

Things to remember that not all blame is on Widget side AFAIR there were other features that made <select> inside UIElement not usable.

The solution is to make some events transparent to CKEditor since we cannot call evt.stopPropagation().

First thought (looks kinda bad but kinda OK at the same moment): decorate event with some uniqally enough property (Symbol are still a thing?):

import {
    makeEventTransparentForCKEditor
} from '@ckeditor/ckeditor5-utils/dom/transparentevents';

domElement.addEventListener( 'focus', ( ev t) => {
    makeEventTransparentForCKEditor( evt );
}, { capture: true } );

// let's say something like this;
makeEventTransparentForCKEditor( evt ) {
    evt.__ckEventPreventDefault = true; // or evt[ eventSymbol ] = true;
};

or even:

import { hideEventsFromTheEditor } from '@ckeditor/ckeditor5-utils/dom/transparentevents';

hideEventsFromTheEditor( domElement );

The above will add previous listener for all events that are handled by CKEditor ('click', 'mousedown', etc).

The editor features will skip such events or maybe event better our DomEmitterMixin could skip events from firing if this symbol is set.

Related issue: #4465. The workaround posted there may actually be resolving the issues mentioned in this ticket.

Idea: Use data-ck-ignore-events on such an element.

I pushed https://github.com/ckeditor/ckeditor5/compare/i/4600 with a prototype and it works just fine. Better than I thought.

And I found a better workaround than what we had in #4465:

[
    'click', 'mousedown', 'mouseup', 'mousemove', 'paste', 'cut', 'copy', 'drop', 'focus', 'blur', 'beforeinput', 'keydown', 'keyup'
].forEach( eventName => {
    editor.editing.view.document.on( eventName, ( evt, data ) => {
        if ( data.domTarget.matches( '[data-cke-ignore-events], [data-cke-ignore-events] *' ) ) {
            evt.stop();
        }
    }, { priority: 999999999 } );
} );

It's still not as reliable as what we can introduce inside observers (e.g. does not cover focus or selection, although, I don't know how important are those), but it seems to work in cases that I tested.

but it seems to work in cases that I tested.

Did you test it within a React app?

Yes, I did prototype it with a React app.

I have a problem with adding support for <select> elements. <input> elements work fine when combined with data-cke-ignore-events. But for some reason, on FF only, <select> does not work – it does not open the options panel on click. No events are caught by DomObserver so the check isn't even executed.

I thought that it may be a problem with native contentEditable=true/false, but it works fine in https://jsfiddle.net/5d761g4j/. So, there's something that we do differently. Perhaps some listener that I'm not aware of or some styling that's applied in the editor. Unfortunately, this will require more thorough investigation.

Oh, I think I know the answer. The fix that I did for selectionchange is stupid. Selectionchange's target is always the document. And I've just added code that check whether evt.target is an element and ignore the event only if it is (as I could not use target.matches() when the target is the document). The check for selectionchange must be smarter there – I need to get selection's common ancestor (or focus/anchor) and do the check for that node to ignore selection change events that happen inside [data-cke-ignore-events].

Doh, blocking selection change does not have any effect on this.

I also checked that blocking the renderer does not help either. And that we don't style the <select> element in a strange way too.

Finally, I checked that DomEmitterMixin doesn't catch any other event that I might've been unaware of. 

To debug this further, we'd need to start stripping next layers of what CKEditor 5 adds over contentEditable, but this will be extremely time-consuming.

For now, <select> elements will not be therefore handled on Firefox.

@Reinmar Did you get a chance to test with <button> element? I tried adding a mouse click handler to a button and a div. Both are not working.

Regarding the documentation part:

🎉

Was this page helpful?
0 / 5 - 0 ratings