Support

Account

Register Login

Forum Replies Created

  • Just tested the hotfix: works great!

  • Sounds good, thanks for the quick response!

    I ended up diagnosing the issue by doing a diff of all changes between 5.9.1 and 5.9.2, and slowly reverting changes back to 5.9.1 until I saw the problem disappear. Here’s the gist of just the relevant changes, if needed:
    https://gist.github.com/figureone/66fb7004596dc611ae7b3165193461ac

    It looks like you were refactoring to better split up changes to options vs. meta, by relying on the existence of "update_{$type}_meta to determine whether to do a meta update or an options update.

    If you want to keep that style, you can maybe use if ( get_meta_table( $type ) ) instead, and then follow with a call to update_metadata( $type, ... ) if it’s true, and update_option() if not. (Similar for the get and delete variants.)

    Looks like that’s how core does the check in update_metadata():
    https://developer.wordpress.org/reference/functions/update_metadata/#source

  • Aloha Elliot,

    I looked up the docs and found that the problem is caused by a subtle difference between update_post_meta() and update_metadata(). In version 5.9.2, you switched to using update_{$type}_meta() in includes/acf-meta-functions.php, but these wrapper functions do not allow updating meta on revisions. See the comment in the core source:
    https://developer.wordpress.org/reference/functions/update_post_meta/#source

    This also applies to delete_post_meta(), FYI:
    https://developer.wordpress.org/reference/functions/delete_post_meta/#source

    So basically when you switched, all acf_update_metadata() and acf_delete_metadata() calls now operate on the parent post if a revision ID is specified. I would suggest reverting to update_metadata() and delete_metadata() to avoid this.

    https://developer.wordpress.org/reference/functions/update_post_meta/
    https://developer.wordpress.org/reference/functions/update_metadata/

  • Aloha Elliot,

    I have traced the issue (on vanilla WordPress) to a change in version 5.9.2 to includes/acf-meta-functions.php. The result is that ACF postmeta does not get created for revision IDs in the database. This happens whether you are saving a draft, or updating a published page. This means:
    * if you try to preview an unpublished revision, you won’t see any ACF fields (the original problem in this thread);
    * if you try to browse revisions, you will see empty ACF postmeta for all the revisions
    * if you try to restore a revision, it won’t have any ACF postmeta associated with it, causing the post’s ACF fields to be deleted.

    Because of that 3rd point, this is probably higher severity. Here’s the diff that causes the issue in acf_update_metadata():

    @@ -240,17 +238,14 @@ function acf_update_metadata( $post_id = 0, $name = '', $value = '', $hidden = f
    return false;
  }

- // Update option.
- if( $type === 'option' ) {
-
+ // Determine CRUD function.
+ if( function_exists("update_{$type}_meta") ) {
+   return call_user_func("update_{$type}_meta", $id, "{$prefix}{$name}", $value);
+ } else {
    // Unslash value to match update_metadata() functionality.
    $value = wp_unslash( $value );
    $autoload = (bool) acf_get_setting('autoload');
    return update_option( "{$prefix}{$id}_{$name}", $value, $autoload );
-
- // Update meta.
- } else {
-   return update_metadata( $type, $id, "{$prefix}{$name}", $value );
  }
 }

    I can confirm that reverting the change above restores functionality (i.e., ACF saves postmeta to revision IDs when saving/updating a post).

    These are related changes (to acf_get_metadata() and acf_delete_metadata()), just in case they are relevant:

    
diff --git a/includes/acf-meta-functions.php b/includes/acf-meta-functions.php
index 1d26f03..9e8c81a 100644
--- a/includes/acf-meta-functions.php
+++ b/includes/acf-meta-functions.php
@@ -196,14 +196,12 @@ function acf_get_metadata( $post_id = 0, $name = '', $hidden = false ) {
    return null;
  }

- // Check option.
- if( $type === 'option' ) {
-   return get_option( "{$prefix}{$id}_{$name}", null );
-
- // Check meta.
- } else {
-   $meta = get_metadata( $type, $id, "{$prefix}{$name}", false );
+ // Determine CRUD function.
+ if( function_exists("get_{$type}_meta") ) {
+   $meta = call_user_func("get_{$type}_meta", $id, "{$prefix}{$name}", false);
    return isset($meta[0]) ? $meta[0] : null;
+ } else {
+   return get_option( "{$prefix}{$id}_{$name}", null );
  }
 }

@@ -286,14 +281,12 @@ function acf_delete_metadata( $post_id = 0, $name = '', $hidden = false ) {
    return false;
  }

- // Update option.
- if( $type === 'option' ) {
+ // Determine CRUD function.
+ if( function_exists("delete_{$type}_meta") ) {
+   return call_user_func("delete_{$type}_meta", $id, "{$prefix}{$name}");
+ } else {
    $autoload = (bool) acf_get_setting('autoload');
    return delete_option( "{$prefix}{$id}_{$name}" );
-
- // Update meta.
- } else {
-   return delete_metadata( $type, $id, "{$prefix}{$name}" );
  }
 }

  • Aloha Elliot,

    I have traced the issue (on vanilla WordPress) to a change in version 5.9.2 to includes/acf-meta-functions.php. The result is that ACF postmeta does not get created for revision IDs in the database. This happens whether you are saving a draft, or updating a published page. This means:
    * if you try to preview an unpublished revision, you won’t see any ACF fields (the original problem in this thread);
    * if you try to browse revisions, you will see empty ACF postmeta for all the revisions
    * if you try to restore a revision, it won’t have any ACF postmeta associated with it, causing the post’s ACF fields to be deleted.

    Because of that 3rd point, this is probably higher severity. Here’s the diff that causes the issue in acf_update_metadata():

    @@ -240,17 +238,14 @@ function acf_update_metadata( $post_id = 0, $name = '', $value = '', $hidden = f
    return false;
  }

- // Update option.
- if( $type === 'option' ) {
-
+ // Determine CRUD function.
+ if( function_exists("update_{$type}_meta") ) {
+   return call_user_func("update_{$type}_meta", $id, "{$prefix}{$name}", $value);
+ } else {
    // Unslash value to match update_metadata() functionality.
    $value = wp_unslash( $value );
    $autoload = (bool) acf_get_setting('autoload');
    return update_option( "{$prefix}{$id}_{$name}", $value, $autoload );
-
- // Update meta.
- } else {
-   return update_metadata( $type, $id, "{$prefix}{$name}", $value );
  }
 }

    I can confirm that reverting the change above restores functionality (i.e., ACF saves postmeta to revision IDs when saving/updating a post).

    These are related changes (to acf_get_metadata() and acf_delete_metadata()), just in case they are relevant:

    diff --git a/includes/acf-meta-functions.php b/includes/acf-meta-functions.php
index 1d26f03..9e8c81a 100644
--- a/includes/acf-meta-functions.php
+++ b/includes/acf-meta-functions.php
@@ -196,14 +196,12 @@ function acf_get_metadata( $post_id = 0, $name = '', $hidden = false ) {
    return null;
  }

- // Check option.
- if( $type === 'option' ) {
-   return get_option( "{$prefix}{$id}_{$name}", null );
-
- // Check meta.
- } else {
-   $meta = get_metadata( $type, $id, "{$prefix}{$name}", false );
+ // Determine CRUD function.
+ if( function_exists("get_{$type}_meta") ) {
+   $meta = call_user_func("get_{$type}_meta", $id, "{$prefix}{$name}", false);
    return isset($meta[0]) ? $meta[0] : null;
+ } else {
+   return get_option( "{$prefix}{$id}_{$name}", null );
  }
 }

@@ -286,14 +281,12 @@ function acf_delete_metadata( $post_id = 0, $name = '', $hidden = false ) {
    return false;
  }

- // Update option.
- if( $type === 'option' ) {
+ // Determine CRUD function.
+ if( function_exists("delete_{$type}_meta") ) {
+   return call_user_func("delete_{$type}_meta", $id, "{$prefix}{$name}");
+ } else {
    $autoload = (bool) acf_get_setting('autoload');
    return delete_option( "{$prefix}{$id}_{$name}" );
-
- // Update meta.
- } else {
-   return delete_metadata( $type, $id, "{$prefix}{$name}" );
  }
 }

  • Aloha Elliot, we noticed this behavior again in latest ACF PRO (5.9.2) using the classic editor. When repeatedly saving a draft, the draft post id will not have any ACF postmeta created (but the main post id will). So you can see the missing fields in the revision browser for the post.

    The workaround mentioned above with removing &preview=true in the preview URL works for us (because it reverts to using the main post id, not the revision id, in the get_field() calls).

    I’ll continue to investigate to see if I can figure out why the revision ID is not also getting the ACF postmeta created. Cheers!

  • Hi Elliot, just checking in to see if you have an update on this issue.

    As a refresher, the current version of ACF will delete backslashes in any wysiwyg field upon saving. This issue exists because the ACF codebase has an extra call to stripslashes_deep() to deal with old PHP versions running magic quotes.

    In my case, this destroys LaTeX markup in those fields, because every LaTeX variable starts with a backslash. Others have chimed in saying that their regular expressions get mangled.

    Your proposed solution seems fine to me. More details on it in your comment here:
    http://support.advancedcustomfields.com/forums/topic/regression-backslashes-stripped-in-wysiwyg/#post-4335

    Thanks!

  • Awesome, I think a back-end filter would be fine, since trying to describe what’s going on here in a UI option might be difficult for nontechnical users to understand.

    For the short term, I’m covered by simply commenting out the stripslashes_deep() call in the ACF code, so it’s not a big rush. Let me know when the filter makes it into core, though! At that point I can roll out a custom filter in all of my relevant themes.

    Cheers,
    -paul

  • Hey @elliot,

    I understand server setups are varied, so lets not waste any more time trying to tackle the problem by identifying which ones have problems with magic quotes.

    Instead, let me show you how ACF doesn’t work on one of the most common server setups, Ubuntu 12.04 LTS (it’s the standard image on Amazon EC2 and linode). Here are some steps I took to reproduce the ACF bug under this setup:

    * I created a test VM environment (Ubuntu 12.04 LTS, PHP 5.3.10, MySQL 5.5.32).
    You can do this easily in vagrant–Ubuntu 12.04 Precise 32bit is the default vagrant box. Instructions on setting up a vagrant virtual machine on your computer is available here:
    http://docs.vagrantup.com/v2/getting-started/

    * I installed a vanilla copy of WordPress 3.6.1 and ACF 4.2.2.

    * I created a new ACF Field Group, and added one Wysiwyg Editor field to the field group. I left all defaults as-is, so the field group appears on all posts on the site.

    * I then edited the sample post included with WordPress (“Hello world!”), and added the following content to the new ACF field:
    Hi! This is ACF content. It just might strip out \ backslashes, which could be \nasty.

    * I then saved the post (by clicking “Update”), and confirmed that the backslashes (“\”) were removed from the above text.

    In short, I think you haven’t heard about this issue from others because the backslash is a lightly used character, and tracing this issue back to ACF is time consuming. But any data loss issue should be treated seriously.

    The easiest solution for you, I think, is to implement another option in the Settings panel for a Wysiwyg field. This setting would simply toggle whether stripslashes_deep() gets called or not. If we do it that way, we don’t have to use get_magic_quotes_gpc() at all, so you shouldn’t get any reports from other users on older PHP setups. This option would default to the current behavior if you wish, so it doesn’t change the default behavior, but it would give me and any others running newer PHP setups the ability to prevent ACF from haphazardly removing backslashes from all ACF wysiwyg content.

    I can provide the pull request with this code, so you don’t have to take extra time writing it. I simply need some sort of assurance that you’ll implement it before I take the time to write the code.

    Cheers,
    -Paul
    PhD Information Science
    College of Education
    University of Hawaii

  • Gah, I should have paid more attention to the commented out section, I didn’t even notice that you had tried my suggestion. Sorry about that.

    Is there any chance you can provide more details on the server setups that have problems? I can try to recreate those setups and see if there’s a better way to detect which specific configurations require stripslashes_deep() and which do not. I’d be willing to invest some time to finding a solution.

    I hesitate to turn magic quotes on on my server because it’s been deprecated in PHP 5.3 and removed in PHP 5.4 (the community has decided it’s a bad way to solve the sanitization problem). I also can’t see a solution where I can hook in after stripslashes_deep() has run in your code, because I have no way of knowing where slashes were removed from. My current solution is to hack your plugin and comment out that line, but (a) that leaves me in the lurch whenever you roll out updates, and (b) it still leaves others with the problem of data loss of backslashes in their ACF wysiwyg content (at least if they’re running newer versions of PHP).

    I suppose a fall back solution is to expose a WordPress option that lets us end users choose whether to have that particular call to stripslashes_deep() enabled. That way you can leave it enabled by default to appease your legacy users, and I (and others) can disable it without hacking the plugin code.

    Let me know which way you’d prefer to go, and I can either (a) research the problem setups with more info from you, or (b) submit a pull request with the code to expose a WordPress option (it would probably appear when editing a field definition that’s set to “Wysiwyg Editor,” under “Show Media Upload Buttons” and “Conditional Logic”).

  • Hey Elliot, sorry for the delay.

    How about using the PHP native function for detecting whether magic quotes is enabled on a particular setup?
    http://php.net/manual/en/function.get-magic-quotes-gpc.php

    If magic quotes is enabled, you can use the stripslashes_deep() call, and skip it otherwise. For example:

    if ( get_magic_quotes_gpc() ) {
  $value = stripslashes_deep( $value );
}

    In place of the offending line:
    https://github.com/elliotcondon/acf/blob/36422e64320045e665b4b98349c4d2bfda7edaa2/core/fields/_functions.php?#L188

  • Thanks for being so responsive, Elliot!

    I’ll let you know if I come up with a solution that (a) doesn’t unnecessarily strip backslashes from wysiwyg field content, and (b) works properly with older PHP setups that have magic quotes enabled.

    If you don’t mind, I’ll leave this issue open until I can report back.

    Cheers,
    -paul

Viewing 12 posts - 1 through 12 (of 12 total)