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

mlContentField insertion order leading to serious data loss #70

Closed
herrvigg opened this issue Jun 21, 2018 · 7 comments
Closed

mlContentField insertion order leading to serious data loss #70

herrvigg opened this issue Jun 21, 2018 · 7 comments
Labels
enhancement New feature or request legacy issue Legacy issue imported from original repo

Comments

@herrvigg
Copy link
Collaborator

Issue by funkjedi
Thursday Mar 05, 2015 at 23:50 GMT
Originally opened as qTranslate-Team/qtranslate-x#70


When qTranslate-X adds content hooks qtranxj_ce() inserts the mlContentField at the top of the form. This results in the fields being added in reverse order. Under normal circumstances this has no adverse affects.

However it can have serious affects when the field names utilize the array notation. For example:

<input name="acf[field][0][field_2]" type="hidden" class="hidden" value="">
<input name="acf[field][0][field_1]" type="hidden" class="hidden" value="">
<input name="acf[field]" type="hidden" class="hidden" value="">

As you can see the third field in the DOM essentially wipes out the values of the other fields when the form is loaded into the $_POST array by PHP causing some serious data loss.

I've worked around it for now by hooking qtranxj_ce() and manually reordering the DOM nodes. It's a pretty ugly hack but it works for now.

(function($) {

    // save reference to function and then
    // replace with a hooked version
    var _qtranxj_ce = qtranxj_ce;
    qtranxj_ce = function(tagName, props, pNode, isFirst) {

        // call original function
        var el = _qtranxj_ce(tagName, props, pNode, isFirst);

        // detach ACF related mlContentField inputs from DOM
        var form = $(pNode);
        var fields = form.children('input[name^=acf]:hidden').detach().toArray();

        // sort mlContentField inputs by input[name]
        fields.sort(function (a,b) {
            if (a.name > b.name) {
                return 1;
            }
            if (a.name < b.name) {
                return -1;
            }
            return 0;
        });

        // insert ACF related mlContentField inputs back into DOM
        form.prepend(fields);

        // return element created by original function
        return el;

    };

})(jQuery);
@herrvigg herrvigg added enhancement New feature or request legacy issue Legacy issue imported from original repo labels Jun 21, 2018
@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Friday Mar 06, 2015 at 20:35 GMT


Tim, there is a compatibility problem with "Visual Composer" and other "Visual Whatever": qTranslate-Team/qtranslate-x#27. It seems to me the current design of mlContentField related things will have to be changed soon to enable better compatibility. We may not rename fields anymore in future. That is why it is better not to rely on the low level specifics of the code and only use high level interface. We can change the high level interface to accommodate your needs.

On this particular issue, I never knew that order actually matters, although it is kind of making sense, after you mentioned it. What if we always append to the end of form? This will keep the order. Can you think of anything which may get broken if we do that?

@herrvigg
Copy link
Collaborator Author

Comment by funkjedi
Saturday Mar 07, 2015 at 00:04 GMT


I think always appending to the end of the form would work great. The fields would still be direct descendants of the form so I can't imagine anything would break.

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Saturday Mar 07, 2015 at 04:37 GMT


I also do not see what can break, so the latest on github puts it at the end. Is there still anything you need to change on -X?

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Tuesday Mar 10, 2015 at 04:41 GMT


Hi Tim, I am about to release next update based on the latest at github. Are you all right? Can you do all you need?

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Tuesday Mar 10, 2015 at 19:24 GMT


I assume it is closed. Please, open new one if there is more to worry about.

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Tuesday Mar 31, 2015 at 18:19 GMT


Hi Tim (@funkjedi), I cannot believe it, but putting fields to the end of form gave trouble to one of the users: https://wordpress.org/support/topic/problems-with-exisiting-posts. If you are in the mood to solve puzzles, I can give you access to the user test site. Indeed, the post does not get saved, when "content" input field is the last one in the form. But it only a trouble for some posts, and not the other. Can you imagine that? I did not dig it into the WP code, there must be some explanation.

For now, I changed common.js again to insert hidden fields right before their originals. This should also keep the order for you. Could you test please, the latest on github?

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Wednesday Apr 01, 2015 at 22:27 GMT


Tim, I think the order would be fine for you, what I more worry about is that those hidden input fields in the middle will add some spaces on the view, although, in theory, they should not. You have pretty complex pages, and I hope that you would notice some layout changes. Would you find time to test it before I break a few thousands of users ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request legacy issue Legacy issue imported from original repo
Projects
None yet
Development

No branches or pull requests

1 participant