Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The clipboardInput event should always be stopped so we always call stopPropagation #6464

Closed
Reinmar opened this issue Mar 20, 2020 · 1 comment · Fixed by ckeditor/ckeditor5-clipboard#68
Assignees
Labels
package:clipboard support:3 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented Mar 20, 2020

📝 Provide a description of the improvement

In #2837 it seems that we only covered the  case when an image is being dropped into the editor. We added this code:

https://github.com/ckeditor/ckeditor5-clipboard/blob/4141ef641dd96defbce5c542401a16c2bebecd8f/src/clipboardobserver.js#L56-L61

And it works since imageuploadediting.js do call evt.stop() when there are files in the clipboard.

However, on normal paste, we don't stop the event. Stopping it in clipboard.js (and changing the priority of that listener from low to lowest cause low doesn't make sense if we always stop it) seems reasonable.

At the same time, why don't we stop propagation of all mouse events, keyboard events, and so on? Why do we talk only about the clipboard? Theoretically we could stop all of the events in all cases cause if something happened in the editor it should be handled in the editor. But then... we don't have observers for all kind of events – we listen only to events that we need to. And that would mean that depending on a feature set, the editor would block some events or not. 

Ideally, taken all that, the editor should not stop the propagation of any event. It's the job of the listener to verify where the event occurred and react or not. However, I know that this is controversial and probably not a way to go too.

So, it seems that the only way around this is to hear the feedback and stop propagation of the events which cause issues to integrators. So far we were notified about clipboard events only by more than one party,  so that makes sense.

@Reinmar Reinmar added type:improvement This issue reports a possible enhancement of an existing feature. package:clipboard labels Mar 20, 2020
@Reinmar Reinmar added this to the iteration 31 milestone Mar 20, 2020
@niegowski niegowski self-assigned this Mar 20, 2020
@niegowski
Copy link
Contributor

We can't stopPropagation of all drop events because there are cases when that events should propagate - #2837. We should only stopPropagation of handled events.

@lslowikowska lslowikowska added the support:1 An issue reported by a commercially licensed client. label Mar 20, 2020
Reinmar added a commit to ckeditor/ckeditor5-clipboard that referenced this issue Mar 25, 2020
Other: Handled `paste` and `drop` events no longer propagate up the DOM tree. Closes ckeditor/ckeditor5#6464.
@lslowikowska lslowikowska added support:3 An issue reported by a commercially licensed client. and removed support:1 An issue reported by a commercially licensed client. labels Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:clipboard support:3 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants