WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 4 months ago

#29920 new defect (bug)

Autosave breaks if post revision fields contains a value that isn't a string

Reported by: mattheu Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Revisions Keywords: has-patch
Focuses: Cc:
PR Number:

Description

This isn't exactly a bug...

Its possible to filter '_wp_post_revision_fields' to add more fields. I am using this to track revisions for some meta fields.

However the value of this must be an string otherwise you get notices when trying to create an autosave - as this calls normalize_whitespace on an array.

It would be nice if this didn't completely break things. I've added a patch that only calls normalize whitespace if both values are strings - otherwise just do a simple comparison.

If you think i'm doing_it_wrong - feel free to tell me so!

I guess this is related to #20564 and any other meta revision fields.

Attachments (3)

29920.diff (1.1 KB) - added by mattheu 5 years ago.
29920.2.diff (421 bytes) - added by mattheu 5 years ago.
Filter wp_get_revision_ui_diff
29920.3.diff (1.6 KB) - added by mattheu 5 years ago.
Filters for triggering autosave.

Download all attachments as: .zip

Change History (10)

@mattheu
5 years ago

#1 @adamsilverstein
5 years ago

  • Component changed from General to Autosave

Very interesting issue, thanks for raising that. What values where you storing in post meta that you were trying to revision?

I think _wp_post_revision_fields is meant to be used internally, hence the name starting with an underscore.

Can you take a look at the way meta revisioning works in the latest patch on #20564 and see if this solves the issue you are having? With the code there your meta should be revisioned if it changes. A restore mechanism is also implemented.

It would be helpful to have more people who need post meta revisioning testing this patch, and helping push it forward. I have a plan to refactor the code there as a plugin with a few core hooks, for now can you give the patch a try?

#2 follow-up: @mattheu
5 years ago

I think you're right that this is only intended to be used internally. Probably lucky it worked as much as it did.

The use case here is that I have a small plugin for meta & taxonomy revisions. This data may be a string or array. I did look at your patch on #20564 and my plugin is somewhat based on this. I'll comment on that ticket separately with feedback.

I guess the real problem I ran into was to get my meta revisions show in the revisions UI. Thinking about it a bit more - I think that a better solution would be to make the value returned by wp_get_revision_ui_diff filterable.

A separate issue is the triggering of autosaves/revisions for meta changes. This actually happens in a few places, and again a better solution might be an action/filter that could be used to handle this.

I would love to see something like this in core - and I think that pushing for better hooks that allow for a solid plugin solution would be a good way forward.

@mattheu
5 years ago

Filter wp_get_revision_ui_diff

@mattheu
5 years ago

Filters for triggering autosave.

#3 @mattheu
5 years ago

I've added a patch for a filter on wp_get_revision_ui_diff

I have also added a patch for a fitler that could be used to trigger the autosave. Slight complexity here is the mixed use of arrays & objects for post data.

I realize this has changed direction from the original ticket - should I leave this open or close and open a new ticket?

#4 @iseulde
5 years ago

  • Component changed from Autosave to Revisions
  • Keywords has-patch added

#5 @mboynes
4 years ago

As it turns out, this comes up in a few places:

  1. wp-admin/includes/post.php (as originally noted in this ticket)
  2. wp-includes/revision.php
  3. wp-admin/edit-form-advanced.php

Furthermore, the current code assumes that every field is singular (calling $post->$meta_key triggers a `get_post_meta()` call with `true` as the third param). If you have more than one meta value for a given key, only the first will be used and the rest ignored.

I think the ultimate solution to this ticket needs to be a bit more robust to cover all of these issues, while also DRYing up this code.

#6 in reply to: ↑ 2 @adamsilverstein
3 years ago

Replying to mattheu:

I guess the real problem I ran into was to get my meta revisions show in the revisions UI. Thinking about it a bit more - I think that a better solution would be to make the value returned by wp_get_revision_ui_diff filterable.

@mattheu I am working on displaying revisions in the revisions UI in the wp-post-meta-revisioning pligning; just starting, appreciate any feedback/testing:

https://github.com/adamsilverstein/wp-post-meta-revisions/tree/features/display-meta-on-revisions-ui

#8 @adamsilverstein
9 months ago

Fine closing this one.

Note: See TracTickets for help on using tickets.