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

When Body uses CKE5, additional Body entry fields are added to the form when clicking "Add another" for a different field #6347

Closed
argiepiano opened this issue Dec 25, 2023 · 9 comments · Fixed by backdrop/backdrop#4618

Comments

@argiepiano
Copy link

argiepiano commented Dec 25, 2023

Description of the bug

If your content type has a field (such as a Text (short) field) with unlimited cardinality, clicking "Add another" will add a new field to both, the Body field (!!) and the unlimited Text (short) field.

Steps To Reproduce

  1. On a clean install with the dev version of Backdrop (so that you have CKE5 as default in core), edit the Page content type and add a Text (short) field with unlimited cardinality, call it "My text"
  2. Visit the add content form for Page
  3. Click "Add another" for the "My text" field

Actual behavior

Additional widgets are added to both, the "My text" field and the "Body" field.

Expected behavior

An additional widget should be added to "My text" field only.

Additional information

Add any other information that could help, such as:

  • Backdrop CMS version: current dev version slated to be released as 1.27.0
  • PHP version: 8.1.13
@argiepiano argiepiano changed the title When Body uses CKE5, new fields are added when clicking "Add another" for a different field When Body uses CKE5, additional Body entry fields are added to it when clicking "Add another" for a different field Dec 25, 2023
@argiepiano argiepiano changed the title When Body uses CKE5, additional Body entry fields are added to it when clicking "Add another" for a different field When Body uses CKE5, additional Body entry fields are added to the form when clicking "Add another" for a different field Dec 25, 2023
@argiepiano
Copy link
Author

This behavior exists also when any widget has any AJAX-enabled action. For example, when you have a File field in the node and select a file. The Body field gets duplicated every time.

@argiepiano
Copy link
Author

argiepiano commented Dec 26, 2023

A bit of digging here: the Backdrop.editors.ckeditor5.detach method supposedly should destroy the editor. There is an if statement in that method that only destroys the editor if the trigger is not 'serialize'.

The problem here is that AJAX requests will pass a 'serialize' trigger to the detach methods every time, as explained in the documentation of backdrop.js.

Since the editor is not destroyed after ajax submission, it remains in the DOM, and a new one is created when the ajax response is processed.

An obvious solution would be to get rid of that if statement, but I don't know enough about CKEditor to figure out if this may create other issues.

Another possible solution is to check if the editor already exists in the dom before creating it (in attach).

Incidentally, only the first "Body" field is passed to the form handlers.

@argiepiano
Copy link
Author

argiepiano commented Dec 26, 2023

Here's a PR: backdrop/backdrop#4618

I decided to go for the second option: avoid creating a CKE5 editor if it already exists on the page. I tried option 1 (always destroying the editor) but that creates some visual issues, since, when the editor is destroyed, the page temporarily shows the HTML markup for a second or two before the ajax response is processed.

@indigoxela
Copy link
Member

indigoxela commented Dec 26, 2023

@argiepiano something similar has been reported recently #6344

But in this case I can reproduce it.

@indigoxela
Copy link
Member

indigoxela commented Dec 26, 2023

the Backdrop.editors.ckeditor5.detach method supposedly should destroy the editor

Not necessarily. The "serialize" trigger, for instance, should allow the editor to write text to the hidden textarea form item.

I'm not sure, why the attach runs for the unrelated body field outside the ajax container... 🤔 Seems to be normal behavior.

However, I can confirm that preventing the additional attach, when there's already an attached editor works properly.

@indigoxela
Copy link
Member

Setting milestone to 1.27 - CKE5 will be part of it.

BTW, weird, that I didn't see the same odd behavior with the other issue (related to paragraphs).

@argiepiano
Copy link
Author

I'm not sure, why the attach runs for the unrelated body field outside the ajax container... 🤔 Seems to be normal behavior.

Thanks @indigoxela. Yes, in my experience, I've seen all attach methods currently loaded in the browser run when ajax requests are processed.

@indigoxela
Copy link
Member

@argiepiano many thanks for the comment update. Much better. 👍

Re attaching multiple editors to the same form element:
Seems like this is another thing that v4 took care of, but v5 doesn't check anymore.

@quicksketch
Copy link
Member

Thanks @argiepiano and @indigoxela! I merged backdrop/backdrop#4618 into 1.x for 1.27.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants