Shift+Enter inserts a <br> even though ShiftEnter plugin is disabled

[jodator]

Is this a bug report or feature request? (choose one)

🐞 Bug report

💻 Version of CKEditor

latest master, Chrome

📋 Steps to reproduce

1. I've only reproduced it on image plugin manual tests (caption namely)
2. Go to the end of
   • paragraph
   • caption
3. Press shift+enter.

✅ Expected result

Either nothing happens either <br> element is inserted.

❎ Actual result

There are two <br>s inserted. Consecutive shift+enter

📃 Other details that might be useful

You cannot place selection there, after typing the <br>s are removed.

[Reinmar]

That's because all the manual tests in this package load only Enter without ShiftEnter.

However, it's strange that <br> isn't filtered out. If ShiftEnter is not loaded, it should be dropped completely. Shift+Enter should not change anything.

[Reinmar]

There seem to be at least two bugs.

Enter must always be handled

The native action Enter must always be prevented, even when just one of the plugins is loaded. However, now, both plugins abort their listeners if Shift is/isn't pressed. Instead, they should prevent the default action before aborting.

This is something I anticipated but ignored to not stale the initial implementation of ShiftEnter :D I hoped we won't see the bug report that soon.

And we wouldn't see it if for the second bug

Why isn't <br> removed from the DOM?

Browser may be doing a many DOM changes as it wants, but we are always re-rendering the DOM to match our view. In this case, the view is clean and contains text only. Then why does the DOM contain two <br>s?

My guess would be that we filter out mutations with bogus <br>s inside the MutationObserver because we treat them as insignificant. And this is true – they are insignificant (we do not represent bogus <br>s in the view). But we still have to re-render the modified element (in this case <figcaption>). It may be enough to call https://docs.ckeditor.com/ckeditor5/latest/api/module_engine_view_renderer-Renderer.html#function-markToSync.

[Reinmar]

Didn't we fix this?

[pomek]

PRs are still open :D

[Reinmar]

OMG... Sorry :D