WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 11 months ago

#11049 accepted defect (bug)

Page Preview does not autosave page template

Reported by: janeforshort Owned by: nacin
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8.4
Component: Autosave Keywords: dev-feedback needs-refresh
Focuses: administration Cc:

Description

When editing a published page, if you change the page template and then click Preview, the preview does not show the new template choice.

Attachments (3)

preview_template.diff (2.1 KB) - added by Nano8Blazex 6 years ago.
patch -> for Google Code In
11049.diff (2.7 KB) - added by nacin 5 years ago.
Different route, hook into get_metadata.
11049.2.diff (2.9 KB) - added by nacin 5 years ago.

Download all attachments as: .zip

Change History (32)

#1 @scribu
7 years ago

  • Milestone changed from Unassigned to 2.9

#2 @ryan
7 years ago

  • Milestone changed from 2.9 to Future Release

#3 @akhilasuram
7 years ago

  • Owner set to akhilasuram
  • Status changed from new to accepted

@Nano8Blazex
6 years ago

patch -> for Google Code In

#4 @Nano8Blazex
6 years ago

  • Keywords has-patch added

#5 @westi
6 years ago

  • Keywords 3.2-early gci added; autosave removed
  • Owner changed from akhilasuram to westi
  • Status changed from accepted to assigned

#6 @westi
5 years ago

  • Keywords 3.2-early removed
  • Milestone changed from Future Release to 3.2

I would like to include this in 3.2

Need to review if we need this much code or can just set the normal post_meta key on the revision that is going to be previewed.

#7 @nacin
5 years ago

Indeed. The patch works only due to a bit of a glitch -- update_post_meta() on a revision post ID ends up attaching the metadata to the post, not the revision. Calling update_metadata() on the revision ID itself is fine, we just have to then make sure we check the revision meta key using get_metadata().

#8 follow-up: @azaozz
5 years ago

That's not a glitch, it's a feature :)

This came up shortly after we added revisions, in some circumstances post meta was attached to the revision and not the actual post, so the meta was more or less "lost".

#9 in reply to: ↑ 8 @nacin
5 years ago

Replying to azaozz:

That's not a glitch, it's a feature :)

Oh, indeed. The patch itself has a glitch, in that it attempts to add post meta to the revision, but then pulls it from be post. Which works due to our API, but I'd think it'd be better to add it to the post directly, otherwise it masks what's going on.

The lower level API will never change, so it's a convenient workaround.

#10 @nacin
5 years ago

Tested, seemed to work well. Made a few tweaks.

Then decided to try this a different way, by storing the value with the revision and getting core to recognize the revised meta value.

When I tested this, I discovered what the showcase template in Twenty Eleven really looked like. The initial patch broke is_page_template() and what not, so going lower in the stack definitely makes sense.

It would have been easier if I believed I should trust $_GET['preview_id'] or that I could easily pass the ID from _set_preview() to _show_page_template_preview(). Used get_the_ID() and is_preview() instead and it's working well here.

Possibility there should be a logic tweak or two to be had here. Please review.

@nacin
5 years ago

Different route, hook into get_metadata.

@nacin
5 years ago

#11 @johnjamesjacoby
5 years ago

Tested 11049.2.diff on twentyten and twentyeleven.

Confirmed working as intended.

#12 @westi
5 years ago

  • Keywords 3.3-early added
  • Milestone changed from 3.2 to Awaiting Review
  • Owner changed from westi to nacin

Missed the boat for 3.2.

Reconsider for 3.3

#13 @nacin
5 years ago

  • Milestone changed from Awaiting Review to Future Release

#14 @nacin
5 years ago

  • Status changed from assigned to accepted

#15 follow-up: @jane
5 years ago

In new version, please save all meta before preview so all info is correct and up-to-date.

#16 in reply to: ↑ 15 ; follow-up: @westi
5 years ago

Replying to jane:

In new version, please save all meta before preview so all info is correct and up-to-date.

In an ideal world this is the solution.

I wonder if this could get tricky though and will fill the db with a lot of duplicate data.

It is going to be an interesting problem to solve and I think it is the right thing to do so as to ensure that Preview is Preview and all the things you might have changed are previewed.

#17 @johnbillion
5 years ago

  • Cc johnbillion@… added

#18 in reply to: ↑ 16 @johnbillion
5 years ago

Replying to westi:

Replying to jane:

In new version, please save all meta before preview so all info is correct and up-to-date.

In an ideal world this is the solution.

I wonder if this could get tricky though and will fill the db with a lot of duplicate data.

This route will of course make the post meta table explode in size. I wonder whether we could serialize all the meta data for each post revision and store it in one meta field.

The latest revision for each post could have its meta data copied over (so it can be used to display the correct post template in previews, etc) and every time a new revision is created we serialize the meta data in the previous revision and store it all in one meta field. This eliminates all the additional rows we'd otherwise see in the post meta table, and it also potentially allows us to revert a post's meta data along with everything else when a post is reverted to a previous revision.

Thoughts?

#19 @jane
5 years ago

  • Keywords gci 3.3-early removed
  • Milestone changed from Future Release to 3.4

#20 @nacin
5 years ago

I like johnbillion's approach, but I would probably just make it save metadata for a preview revision and nothing else. Looking back 8 months later, I don't like my patch for that one reason — it looks like it saves the page template for all revisions, rather than just preview saves.

#21 @nacin
5 years ago

  • Keywords dev-feedback added

#22 @mbijon
5 years ago

  • Cc mike@… added

What if we create a _wp_revision_template meta value that is only used and updated by previews?

If it's tied to the post instead of the revision we could only have one per-post, solving the duplicates issue. I'm assuming it's unlikely people would need to store the template per revision or per-preview.

This fixes the need to have meta on revisions, but does open the door for adding excess meta to posts.

#23 @ryan
4 years ago

  • Milestone changed from 3.4 to Future Release

#25 @MikeHansenMe
4 years ago

  • Component changed from Administration to Autosave

#26 @SergeyBiryukov
3 years ago

#25276 was marked as a duplicate.

#27 @adamsilverstein
3 years ago

  • Cc adamsilverstein@… added

#28 @chriscct7
11 months ago

  • Focuses administration added
  • Keywords needs-refresh added; has-patch removed

Replicated on 4.3.1

#29 @adamsilverstein
11 months ago

Related: #20299, #20564.

With the post meta revisions plugin and the last patch on #20299, adding _wp_page_template to the revisioned whitelist using the wp_post_revision_meta_keys filter would fix this issue. The _wp_page_template meta would be revisioned and included in previews.

Another reason we should add revisioning of post meta to core.

Note: See TracTickets for help on using tickets.