Opened 10 years ago
Last modified 6 years ago
#29920 new defect (bug)
Autosave breaks if post revision fields contains a value that isn't a string
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Revisions | Keywords: | has-patch |
Focuses: | Cc: |
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)
Change History (10)
#2
follow-up:
↓ 6
@
10 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.
#3
@
10 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?
#5
@
10 years ago
As it turns out, this comes up in a few places:
- wp-admin/includes/post.php (as originally noted in this ticket)
- wp-includes/revision.php
- 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
@
9 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:
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?