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

ACF wysiwyg standard editor broken #1186

Closed
degorov-ru opened this issue Jun 9, 2022 · 15 comments
Closed

ACF wysiwyg standard editor broken #1186

degorov-ru opened this issue Jun 9, 2022 · 15 comments
Labels
module: ACF Integration with ACF severity: major Major functionality

Comments

@degorov-ru
Copy link

degorov-ru commented Jun 9, 2022

Hello.
I use the qtranslate-xt translation plugin. It works fine with ACF version 5.8.9. With all versions newer, it has an error in the wysiwyg. The error is that when switching the language, the content does not change.

ACF developers answered me:

From our end, we are not able to confirm how the qtranslate-xt plugin handles translation therefore we are not in a better position to conclusively advise on this.

Due to the complexity in translation plugins, we may not be well equipped to advise on this translation-related issue. I would recommend reaching out to the qtranslate-xt support regarding the issue as their plugin handles the translation. Hopefully, they will shed some light on the same.

Please note that our support is limited in scope to the basic usage of the ACF plugin and we don't offer any translation-related support to our customers.

I hope this info helps and please let me know if there's anything else I can help you with.

Video presentation of the problem:
https://disk.yandex.ru/d/yPUcrD7MQoCufw
https://disk.yandex.ru/i/xBPKo-dbdPkdWA

There are no errors in the console.
I launched a clean website, without third-party themes and plugins. Installed the latest version of QT 3.12 and ACF PRO 5.12.2. You can personally verify that it does not work.

Example site
https://degorov.ru/blank/wp-admin/
login: admin
pass: admin

BackUp
https://disk.yandex.ru/d/iA5NVQ4bKn-pFA - files
https://disk.yandex.ru/d/l9D1PFkk58hNSg - database

@herrvigg herrvigg added the module: ACF Integration with ACF label Jun 9, 2022
@herrvigg
Copy link
Collaborator

herrvigg commented Jun 9, 2022

Good to have a test site:

  • ACF "Wysiswyg Editor" is not working ❌
  • ACF "Wysiswyg Editor (qTranslate-XT)" is working ✔️

I don't think the problem with the ACF Wysiswyg editor is new. It's mentioned in #985 and #1044.
See #985 (comment)_. Checking "Enable translation for Standard Field Type" did not help. I don't know about the "hack" with the cache.
Can't you use "Wysiswyg Editor (qTranslate-XT)" until the other one is fixed (if possible)?

@herrvigg herrvigg added the need info The submitter must provide more info label Jun 9, 2022
@degorov-ru
Copy link
Author

Thanks for the solution! Alas, I could not find this solution earlier and experienced difficulties for almost a year. Could you add a note to the instructions for this moment?

If you need help, I can forward your messages to ACF PRO developers to solve this problem.

@herrvigg
Copy link
Collaborator

It's probably due to the way the main ACF wysiwyg editor is initialized. We'd need to investigate first what happens in QTX.

I wouldn't know where to put a note, this a standard field controlled by ACF, not created by QTX. You can see the open the issues on the ACF module searching by tag: https://github.com/qtranslate/qtranslate-xt/labels/plugin%3A%20ACF

@MKRD-SUPPORT
Copy link

I had difficulties with my GitHub account and therefore I had only tested the Wysiwyg Editor (qTranslate-XT) field on the example page (degorov-ru).

I also had to change the Wysiwyg Editors (qTranslate-XT) since ACF(-PRO) version 5.8.9.

I was forced to take this dependency on running projects!

As far as I can tell, the interoperability between the two plugins and their versions is not always guaranteed.

The last working pair was QTX (3.11.4) with ACF-PRO (5.8.9).

Here is the info in connection with Mozilla Firefox
AdvancedCustomFields/acf#531

@mattgrshaw
Copy link

Hey folks, I'm one of the ACF devs and we're happy to help out on this one if we can.

Just getting caught up on this one - it seems that the qtranslate-xt plugin includes a modified version of some ACF fields in order to make the values for those fields translatable, is that correct?

If so, are there any hooks or similar we could provide that might make the integration a bit easier? I'm wondering if it might be possible to use ACF's hooks with existing fields rather than having to create new fields, but I'm sure it's not quite that simple in practice!

@herrvigg herrvigg removed the need info The submitter must provide more info label Jun 18, 2022
@herrvigg
Copy link
Collaborator

Hey folks, I'm one of the ACF devs and we're happy to help out on this one if we can.

Cool, thanks! 😎

Just getting caught up on this one - it seems that the qtranslate-xt plugin includes a modified version of some ACF fields in order to make the values for those fields translatable, is that correct?

Yes. You can find them here, it will surely talk to you:
https://github.com/qtranslate/qtranslate-xt/tree/master/modules/acf/src/fields

If so, are there any hooks or similar we could provide that might make the integration a bit easier?

This module comes from this plugin: https://github.com/funkjedi/acf-qtranslate. I contacted the original author several times but no answer. Since the former plugins (qTranslate and qTranslate-X) have been abandoned, the users base is here anyway (qTranslate-XT), so we can make evolutions in this ACF module. Ideally, hooks could allow to get rid of the derived classes so the standard ACF fields could be enough. That would surely be better for the users to avoid dealing with different sets of fields. But we can first see how to fix the Wysywig editor.

@herrvigg
Copy link
Collaborator

In short, the derived fields are overriding:

  • render_field
  • render_field_settings
  • format_value
  • update_value
  • validate_value

For the wysiwyg editor it's more tricky as qTranslate-XT does some manipulation with the text editor (MCE) to allow editing and switching multiple languages before saving.

@MKRD-SUPPORT
Copy link

Well, since this works in single-language mode, I belive that the dynamic language switching was implemented with faulty logic (JavaScript).

Technically (isolated script) the language switching works with TinyMCE (ACF).

@nalato
Copy link

nalato commented Sep 13, 2022

Hello,
I think I have found a related bug with ACF Pro 5.12.2 and Qtranslate XT 3.12.1 (Qtranslate XT 3.12.0 works fine).
In the ACF options page (/wp-admin/admin.php?page=acf-options) the language tabs for each field are blocked.
In the Wysiwyg Editor also the Visual/Text tabs don't work.
https://ibb.co/ThgJfLT

When i press a Visual/Text tab there is an javascript error:

Uncaught TypeError: r is undefined
    e https://www.blowtherm.it/wp-includes/js/tinymce/tinymce.min.js?ver=49110-20201110:2
    e https://www.blowtherm.it/wp-includes/js/tinymce/tinymce.min.js?ver=49110-20201110:2
    bind https://www.blowtherm.it/wp-includes/js/tinymce/tinymce.min.js?ver=49110-20201110:2
    M https://www.blowtherm.it/wp-includes/js/tinymce/tinymce.min.js?ver=49110-20201110:2
    init https://www.blowtherm.it/wp-includes/js/tinymce/tinymce.min.js?ver=49110-20201110:2
    n https://www.blowtherm.it/wp-admin/js/editor.min.js?ver=6.0.2:2
    e https://www.blowtherm.it/wp-admin/js/editor.min.js?ver=6.0.2:2
    C https://www.blowtherm.it/wp-includes/js/tinymce/tinymce.min.js?ver=49110-20201110:2
    d https://www.blowtherm.it/wp-includes/js/tinymce/tinymce.min.js?ver=49110-20201110:2

Thanks

@herrvigg herrvigg mentioned this issue Nov 5, 2022
herrvigg added a commit that referenced this issue Nov 23, 2022
Major fix for the support of generic translatable ACF settings such
as label, description and default value.

Use undocumented `open_field_object`, works with ACF5.12.4 and ACF6.

Delete obsolete ACF `append_field` JS hook.
Anyway the hook for wysiwyg editor is currently broken (#1186).
@herrvigg
Copy link
Collaborator

It was very difficult to find but I understand the problem now by debugging the ACF code. The problem is due to a change of element id for the wysiwyg fields that they change without any related event... 😓

https://github.com/AdvancedCustomFields/acf/blob/366796a529219c1c5c2722a80371eb56a5fd1425/assets/build/js/acf-input.js#L5090-L5104

The hooks have already been created for the textarea by the generic machinery of qTranslate. It requires some specific handling to fix the hooks and re-attach the wysiwyg textarea and the tinyMCE editor with special cases for visual/html modes.

@herrvigg herrvigg changed the title wysiwyg does not work in the ACF PRO 5.12.2 and qtranslate-xt 3.12 ACF wysiwyg standard editor broken Nov 27, 2022
herrvigg added a commit that referenced this issue Nov 27, 2022
ACF is destroying HTML elements in their `initializeEditor` code.
This results in QTX hooks pointing to detached HTML elements that
no longer exist in the document, breaking the LSB switch.
See: AdvancedCustomFields/acf#767

Change the main i18n configuration to filter out any `acf-editor`
so that the hooks are not created too soon. Delaye them to the
`wysiwyg_tinymce_settings` filter which also sets the mceInit
callback for the visual mode.
@herrvigg
Copy link
Collaborator

The problem with the elements destroyed by ACF is reported in AdvancedCustomFields/acf#767.

Finally I found a workaround for the standard ACF wysiwyg editor, by delaying the creation of the hooks - yay!
Fixed in master so please test, the next release is approaching.

@MKRD-SUPPORT
Copy link

Everything worked during my tests!

@herrvigg
Copy link
Collaborator

Good! I had a doubt leaving the default wpautop = true. This used to be a problem in the past but it seems the new QTX handler is doing the job (updateMceEditorContent).

@herrvigg herrvigg added the severity: major Major functionality label Nov 27, 2022
@herrvigg
Copy link
Collaborator

herrvigg commented Nov 27, 2022

Just getting caught up on this one - it seems that the qtranslate-xt plugin includes a modified version of some ACF fields in order to make the values for those fields translatable, is that correct?
If so, are there any hooks or similar we could provide that might make the integration a bit easier? I'm wondering if it might be possible to use ACF's hooks with existing fields rather than having to create new fields, but I'm sure it's not quite that simple in practice!

@mattgrshaw thanks for your message again. Very good remarks, I agree completely.
So far we have two main methods to support translations in ACF:

  1. ACF standard fields, integrated with our JS code (using ACF JS API)
  2. qTranslate-XT fields extending ACF fields, mostly in PHP (render / format / update).

Imo we should stick to method 1 and deprecate 2.
Adding new fields that are mapped to existing fields create some complications, and some of the PHP code in the QTX module is duplicating part of the ACF code, which is not nice.

Current fields

  1. text (input), textarea, wysiwyg (fixed in this ticket)
  2. file, image, post_object, textarea, text, url, wysiwyig

If we can handle the missing ones in 1 -which I think is possible- we can drop method 2. It will require more JS code but there's some good potential, even to support new fields, and I believe it's more sustainable on the long run.

@herrvigg
Copy link
Collaborator

Released in 3.13.0.

herrvigg added a commit that referenced this issue Mar 30, 2023
Follow-up of #1186 and #1300.
Since the standard ACF fields are better supported, those should be
preferred over the qTranslate extended fields.

In particular: text, textarea, wysiwyg are covered by the ACF standard
fields, while others only exist as qTranslate extended fields.

Deprecate softly these "duplicated" extended fields in their labels.
herrvigg added a commit that referenced this issue Mar 30, 2023
Follow-up of #1186 and #1300.
Since the standard ACF fields are better supported, those should be
preferred over the qTranslate extended fields.

In particular: text, textarea, wysiwyg are covered by the ACF standard
fields, while others only exist as qTranslate extended fields.

Deprecate softly these "duplicated" extended fields in their labels.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: ACF Integration with ACF severity: major Major functionality
Projects
None yet
Development

No branches or pull requests

5 participants