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

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

Open
slotterbackW opened this issue Nov 4, 2022 · 17 comments
Open

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

slotterbackW opened this issue Nov 4, 2022 · 17 comments
Labels
support:1 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@slotterbackW
Copy link

slotterbackW commented Nov 4, 2022

📝 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.

@slotterbackW slotterbackW added the type:bug This issue reports a buggy (incorrect) behavior. label Nov 4, 2022
@urbanspr1nter
Copy link
Contributor

urbanspr1nter commented Nov 4, 2022

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
Copy link
Contributor

urbanspr1nter commented Nov 7, 2022

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
Copy link
Contributor

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
Copy link
Member

Reinmar commented Nov 8, 2022

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
Copy link
Member

Reinmar commented Nov 9, 2022

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
Copy link
Contributor

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: #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
Copy link
Member

Reinmar commented Nov 16, 2022

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

@NachoVazquez
Copy link

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 slotterbackW changed the title Using the Cypress .type() command with the newest version of CKEditor throws an Error Using the Cypress .type() command with version 35.3.0 throws an Error Nov 30, 2022
@slotterbackW slotterbackW changed the title Using the Cypress .type() command with version 35.3.0 throws an Error Using the Cypress .type() command with v35.3.0 throws an Error Nov 30, 2022
@slotterbackW
Copy link
Author

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
Copy link

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
Copy link

chris55miao commented Jan 4, 2023

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
Copy link

chris55miao commented Jan 5, 2023

We have issue using cypress.realType because it does not support non-English typing (dmtrKovalenko/cypress-real-events#290)

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

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

@omairvaiyani
Copy link

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
Copy link

Dragod commented Feb 8, 2023

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")

@aldonace-wu aldonace-wu added the support:2 An issue reported by a commercially licensed client. label Feb 27, 2023
@rdhelms
Copy link

rdhelms commented Mar 8, 2023

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
Copy link
Member

Reinmar commented Mar 8, 2023

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
Copy link

aorioir commented Apr 20, 2023

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 } 
});

@lslowikowska lslowikowska added support:1 An issue reported by a commercially licensed client. and removed support:2 An issue reported by a commercially licensed client. labels Jul 24, 2023
leadegroot pushed a commit to uqlibrary/fez-frontend that referenced this issue Dec 19, 2023
leadegroot pushed a commit to uqlibrary/fez-frontend that referenced this issue Dec 19, 2023
To achieve this, we had to _build from source_
Note we are trapped below the latest version - past about V34, ckeditor crashes in cypress (although it runs fine in localhost) - probably the same cause as in ckeditor/ckeditor5#12802
leadegroot pushed a commit to uqlibrary/homepage-react that referenced this issue Jan 8, 2024
leadegroot pushed a commit to uqlibrary/homepage-react that referenced this issue Jan 9, 2024
leadegroot pushed a commit to uqlibrary/homepage-react that referenced this issue Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
support:1 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests