search

AdvancedCustomFields / acf

Advanced Custom Fields
http://advancedcustomfields.com/
835 stars 171 forks source link

wysiwyg JS `initializeEditor` should not destroy HTML elements #767

Open herrvigg opened 1 year ago

herrvigg commented 1 year ago

hi, we in qTranslate-XT (*) have identified a problem in ACF Javascript with the wysiwyg editor: https://github.com/qtranslate/qtranslate-xt/issues/1186. In short the issue in ACF comes from here the JS initializeEditor code here in acf-input.js: https://github.com/AdvancedCustomFields/acf/blob/366796a529219c1c5c2722a80371eb56a5fd1425/assets/build/js/acf-input.js#L5099-L5104

This code is deleting HTML elements with destructive = true and creating a similar HTML content. It only looks similar which is very misleading and took me a lot of time to debug... Problem: it's not the same HTML content because we are using those HTML elements as object references. So the old references point to detached elements that are not in the document anymore and the new elements created by ACF are unknown, which breaks several functionalities in qTranslate but other plugins integrating ACF could also be impacted.

I have found a workaround but it's not ideal. So here are my suggestions:

  1. this destructive functionality should be avoided and possibly removed completely. The same can be achieved by changing the ID in a soft way. Why did the developers use destructive here?
  2. regardless of destructive or not, ACF should send an event telling such a change of ID happened (it's already a big HTML change) or if the element is destroyed and created again (even a bigger change).

(*) For a short history, qTranslate-XT is quite a big WordPress project with a legacy from the original qTranslate that was abandoned by the original author despite having many thousands of installations. We hope to bring it back to the official plugins soon. We live only in github for now but it's quite popular. We have been integrating ACF in different ways successfully for many users. In fact ACF and WooCommerce are the most succesfull integrations for qTranslate. There's a lot of potential to make these two plugins to coexist very smoothly.

herrvigg commented 1 year ago

Also, why are all these HTML ID changed in the JS code? This is not a good practice, it should be avoided. It's a source of many bugs to change them on the fly at load time. The IDs should be generated once for all by the PHP code.

herrvigg commented 1 year ago

Any update on this? Just to be sure you understand the problem. I came up with a workaround but it's far from ideal.