Using the Cypress `.type()` command with v35.3.0 throws an Error

slotterbackW commented 1 year ago

📝 Provide detailed reproduction steps (if any)

We're super excited about the newest CKEditor release that adds support for the beforeInput event, however, when trying to upgrade we noticed that with the newest version (v35.3.0) many of our Cypress tests have started failing.

It looks like the issue is due to how Cypress's type command interacts with CKEditor. Any call to type(), which includes typing letter keys (i.e. not just {rightArrow}), results in the following error:

TypeError: Cannot read properties of null (reading 'root')
    at deleteContent (deletecontent.js:63:1)
    at Model.deleteContent (model.js:555:1)
    at Model.<anonymous> (observablemixin.js:201:1)
    at Model.fire (emittermixin.js:154:1)
    at <computed> [as deleteContent] (observablemixin.js:204:1)
    at Object.callback (inserttextcommand.js:82:1)
    at Model._runPendingChanges (model.js:895:1)
    at Model.enqueueChange (model.js:245:1)
    at InsertTextCommand.execute (inserttextcommand.js:80:1)
    at InsertTextCommand.<anonymous> (observablemixin.js:201:1)
    at InsertTextCommand.fire (emittermixin.js:154:1)
    at <computed> [as execute] (observablemixin.js:204:1)
    at CommandCollection.execute (commandcollection.js:61:1)
    at DecoupledEditor.execute (editor.js:381:1)
    at Mixin.<anonymous> (input.js:88:1)
    at Mixin.fire (emittermixin.js:154:1)
    at fireListenerFor (bubblingemittermixin.js:130:1)
    at Document.fire (bubblingemittermixin.js:69:1)
    at Mixin.<anonymous> (inserttextobserver.js:51:1)
    at Mixin.fire (emittermixin.js:154:1)
    at fireListenerFor (bubblingemittermixin.js:130:1)
    at Document.fire (bubblingemittermixin.js:69:1)
    at InputObserver.fire (domeventobserver.js:72:1)
    at InputObserver.onDomEvent (inputobserver.js:104:1)
    at listenTo.useCapture (domeventobserver.js:56:1)
    at ProxyEmitter.fire (emittermixin.js:154:1)
    at HTMLDivElement.domListener (emittermixin.js:244:1)

✔️ Expected result

The type command works normally

❌ Actual result

JS error

❓ Possible solution

If you have ideas, you can list them here. Otherwise, you can delete this section.

📃 Other details

Browser: Chrome
OS: macOS Monterey 12.3.1
First affected CKEditor version: 35.3.0

Installed CKEditor plugins:

Essentials,
Paragraph,
BoldEditing,
ItalicEditing,
UnderlineEditing,
CodeEditing,
IndentEditing,
IndentBlock,
BlockQuoteEditing,
ListEditing,
HorizontalLineEditing,
HeadingEditing,
CodeBlockEditing,
LinkEditing,
Image,
ImageResize,
LinkImageEditing,
Table,
PasteFromOffice,
Autoformat

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

urbanspr1nter commented 1 year ago

We are getting the same issue except we are using a Selenium-based driver for our tests. Everything isn't behaving correctly anymore in our automation. Unfortunately this is something which is blocking us from upgrading.

Did all typing scenarios get migrated to beforeinput? This is actually more of a breaking change for us than expected. For example, we read enter event as an assumption of a key event, but this migration to beforeinput ended up breaking many things for us (we lose some of the properties key event gives us). We can solve this specific occurrence, but it looks like the upgrade may have a larger ripple effect with other typing-like events.

urbanspr1nter commented 1 year ago

Actually correction... Seems like some of the quirks from CKE5 that our E2E tests tried to get by was fixed in 35.3.0 after all this time. Looks like migration to beforeinput helped. After removing our "workarounds", things were working as expected again. However, there are still behaviors which changed in that still cause our own automation to fail.

LukaszGudel commented 1 year ago

Hi @slotterbackW,

in our e2e tests written in Cypress we use commands from cypress-real-events package. This package exposes the cy.realType command, which works correctly with CKEditor 5 35.3.0. When using such a command, you only need to ensure that the editor is in focus, and then you can use it as cy.realType( 'some text to type' ); without any additional gets with selectors.

Using cypress-real-events also allows using cy.realHover to test editor behavior on hovers and cy.realPress to use keyboard shortcuts like Cmd/Control + B.

Unfortunately, using the cypress-real-events package also have some drawbacks as they only work on chromium-based browsers.

As for the Selenium-based testing, we are not able to give any insights or tips on how to approach testing CKEditor 5 as we are not using such tools in our tests.

Reinmar commented 1 year ago

Thanks for the feedback and sorry for the inconvenience! I have to admit, I may have underestimated the implication of some of the changes we made. For instance, I completely haven't thought about E2E tests. Perhaps this should've been at least marked as major BC in our changelog.

One issue with this project was that it took so long (~2y from the start) that we lost track of some of the less-apparent breaking changes. So, the changelog may also be missing some parts.

I'll let the team know about this issue thread and we'll keep monitoring other issues as well. So if you'll run into any issues, let us know – we may be able to share some tips (just like @LukaszGudel did above) or even bring back missing things in future releases.

Reinmar commented 1 year ago

Did all typing scenarios get migrated to beforeinput?

Yes, all of them.

We could theoretically bring back mutation handler, but we were happy to actually remove it as it was a complexity hell. And it wasn't super stable either.

Thus, e.g. spell checker uses the input command of CKE5 to provide a native integration.

On the other hand, I suppose that Grammarly doesn't have a CKE5-specific integration (although, I'm not 100% sure). And it works with the newest version of CKE5. Maybe, @niegowski, could you check how it operates?

urbanspr1nter commented 1 year ago

Did all typing scenarios get migrated to beforeinput?

Yes, all of them.

We could theoretically bring back mutation handler, but we were happy to actually remove it as it was a complexity hell. And it wasn't super stable either.

Thus, e.g. spell checker uses the input command of CKE5 to provide a native integration.

On the other hand, I suppose that Grammarly doesn't have a CKE5-specific integration (although, I'm not 100% sure). And it works with the newest version of CKE5. Maybe, @niegowski, could you check how it operates?

Thanks @Reinmar for that insight, and for hearing me out. It is very appreciated. I don't want to steal away the discussion and will try to continue here: https://github.com/ckeditor/ckeditor5/issues/11636 as I think it is more relevant.

It would be great to see how other integrations do it, but unfortunately for us, I'm not sure about the flexibility in accommodation for our CKE5 integration. (This spell checker/grammar correction is used in more than just our application)

Reinmar commented 1 year ago

We moved the discussion about Microsoft's spell/grammar checker to #12844.

NachoVazquez commented 1 year ago

I'm having this problem. From what I can tell the solution right now is to use cypress-real-events.

Is this correct or does exist an official workaround for this?

slotterbackW commented 1 year ago

Thanks @LukaszGudel for the workaround! It was a decent amount of work to migrate our tests to use cypress-real-events since the API is not exactly the same, but we were able to migrate all our tests and upgrade to this version.

I did want to mention though that now that we've upgraded, our logs are showing that many of our users are running into the same error:

TypeError: Cannot read properties of null (reading 'root')
    at deleteContent (deletecontent.js:63:1)
    at Model.deleteContent (model.js:555:1)
    at Model.<anonymous> (observablemixin.js:201:1)
    at Model.fire (emittermixin.js:154:1)
    at <computed> [as deleteContent] (observablemixin.js:204:1)
    at Object.callback (inserttextcommand.js:82:1)
    at Model._runPendingChanges (model.js:895:1)
    at Model.enqueueChange (model.js:245:1)
    at InsertTextCommand.execute (inserttextcommand.js:80:1)
    at InsertTextCommand.<anonymous> (observablemixin.js:201:1)
    at InsertTextCommand.fire (emittermixin.js:154:1)
    at <computed> [as execute] (observablemixin.js:204:1)
    at CommandCollection.execute (commandcollection.js:61:1)
    at DecoupledEditor.execute (editor.js:381:1)
    at Mixin.<anonymous> (input.js:88:1)
    at Mixin.fire (emittermixin.js:154:1)
    at fireListenerFor (bubblingemittermixin.js:130:1)
    at Document.fire (bubblingemittermixin.js:69:1)
    at Mixin.<anonymous> (inserttextobserver.js:51:1)
    at Mixin.fire (emittermixin.js:154:1)
    at fireListenerFor (bubblingemittermixin.js:130:1)
    at Document.fire (bubblingemittermixin.js:69:1)
    at InputObserver.fire (domeventobserver.js:72:1)
    at InputObserver.onDomEvent (inputobserver.js:104:1)
    at listenTo.useCapture (domeventobserver.js:56:1)
    at ProxyEmitter.fire (emittermixin.js:154:1)
    at HTMLDivElement.domListener (emittermixin.js:244:1)

Unfortunately, so far, using Cypress's type() command is the only way I've found to reliably reproduce the issue, but I did want to note that it seems like this error is affecting our users in addition to our tests too.

DerJacques commented 1 year ago

We were running into the same issue using Testcafe.

Our solution was to simulate the beforeinput event, so I'm posting it here for future reference in case anyone else needs it:

const editor = document.querySelector('.ck-editor__editable')

editor.dispatchEvent( new window.InputEvent( 'beforeinput', {
    bubbles: true,
    cancelable: true,
    inputType: 'insertText',
    data: 'Text to insert here',
    targetRanges: [new StaticRange({ startContainer: editor, startOffset: 0, endContainer: editor, endOffset: 0 })]
} ) );

To be honest, I'm far from an expert when it comes to the StaticRange part, but the above code should insert the text at the very top of the editor.

I hope this is helpful.

chris55miao commented 1 year ago

Not sure if thats related, but after I bump CKEditor5 from v35.0.1 to v35.4.0, any UserEvent.keyboard or UserEvent.type (from react @testing-library/user-event) are not working on the CKEditor component anymore. I haven't figured out the workaround yet.

chris55miao commented 1 year ago

We have issue using cypress.realType because it does not support non-English typing (https://github.com/dmtrKovalenko/cypress-real-events/issues/290)

We ended up using the below with @DerJacques 's solution inside

cy.findByTestId("xxx")
    .then(($editor) => {$editor[0].dispatchEvent(...)}

omairvaiyani commented 1 year ago

Our tests have also started failing from v35. We rely on Ember's test-helpers fillIn which no longer finds/fills in text in the editor.

Dragod commented 1 year ago

Is there a fix for this yet? We have upgraded the ckeditor last week and now tests are failing.

Edit: Fixed with the plugin suggested by @LukaszGudel, even though I would prefere a solution without using a plugin.

Replaced cy.get(".myselector").type("something") with cy.get(".myselector").realType("something")

rdhelms commented 1 year ago

Wow, this took me a ton of debugging to finally track down - would love for this to be fixed asap 🙏

This was my eventual workaround for now based on ckeditor's FAQ about DOM element's ckeditorInstance, but really we need .type() to work as expected:

Before

cy.get('.ck-content[contenteditable=true]').type('Typing some stuff')

After

cy.get('.ck-content[contenteditable=true]').then(el => {
    const editor = el[0].ckeditorInstance  // If you're using TS, this is ReturnType<typeof InlineEditor['create']>
    editor.setData('Typing some stuff')
})

Also...I was using ckeditor v36.0.1 and still seeing the issue

Reinmar commented 1 year ago

What I'd propose to do here:

Have CKE5 log a warning (or error if we're sure this is actually always broken) when we detect that CKE5 is being tested/used with incomplete fake events. Explain in the error that some tool (e.g. Cypress or a custom spell-checker) fires this and that and should be improved. We can also mention the workaround with cypress-real-events.
Report the issue to Cypress. It's an upstream issue and we should check whether they are aware of this. I didn't find one (though, didn't look much).

The other option:

Have CKE5 use selection when target ranges aren't specified. This perhaps should be combined with logging a warning when we detect such situation to have any visibility of the problem. Such approach might resolve issues like this one here, but it means that there will be less pressure on the tooling to fix their issues. Also, we cannot guarantee that this will actually work fine (e.g. that the text will appear where expected).

aorioir commented 1 year ago

We were running into the same issue using Testcafe.

Our solution was to simulate the beforeinput event, so I'm posting it here for future reference in case anyone else needs it:
const editor = document.querySelector('.ck-editor__editable')

editor.dispatchEvent( new window.InputEvent( 'beforeinput', {
    bubbles: true,
    cancelable: true,
    inputType: 'insertText',
    data: 'Text to insert here',
    targetRanges: [new StaticRange({ startContainer: editor, startOffset: 0, endContainer: editor, endOffset: 0 })]
} ) );
To be honest, I'm far from an expert when it comes to the StaticRange part, but the above code should insert the text at the very top of the editor.

I hope this is helpful.

Your workaround worked for me, thanks very much. I have realize that you can recover ckeditor object so you can use set method...

here you are another workaround:

await t.eval(() => {
   const editor = window.document.querySelector(".ck-editor__editable");
   editor.ckeditorInstance.data.set(whateverYouWant);
},
{ 
   dependencies: { whateverYouWant } 
});

ckeditor / ckeditor5