WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 4 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: has-patch dev-feedback
Focuses: 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 3 years ago.
patch -> for Google Code In
11049.diff (2.7 KB) - added by nacin 3 years ago.
Different route, hook into get_metadata.
11049.2.diff (2.9 KB) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (30)

comment:1 scribu4 years ago

  • Milestone changed from Unassigned to 2.9

comment:2 ryan4 years ago

  • Milestone changed from 2.9 to Future Release

comment:3 akhilasuram4 years ago

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

Nano8Blazex3 years ago

patch -> for Google Code In

comment:4 Nano8Blazex3 years ago

  • Keywords has-patch added

comment:5 westi3 years ago

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

comment:6 westi3 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.

comment:7 nacin3 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().

comment:8 follow-up: azaozz3 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".

comment:9 in reply to: ↑ 8 nacin3 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.

comment:10 nacin3 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.

nacin3 years ago

Different route, hook into get_metadata.

nacin3 years ago

comment:11 johnjamesjacoby3 years ago

Tested 11049.2.diff on twentyten and twentyeleven.

Confirmed working as intended.

comment:12 westi3 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

comment:13 nacin3 years ago

  • Milestone changed from Awaiting Review to Future Release

comment:14 nacin3 years ago

  • Status changed from assigned to accepted

comment:15 follow-up: jane3 years ago

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

comment:16 in reply to: ↑ 15 ; follow-up: westi3 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.

comment:17 johnbillion3 years ago

  • Cc johnbillion@… added

comment:18 in reply to: ↑ 16 johnbillion3 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?

comment:19 jane2 years ago

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

comment:20 nacin2 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.

comment:21 nacin2 years ago

  • Keywords dev-feedback added

comment:22 mbijon2 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.

comment:23 ryan2 years ago

  • Milestone changed from 3.4 to Future Release

comment:25 MikeHansenMe19 months ago

  • Component changed from Administration to Autosave

comment:26 SergeyBiryukov7 months ago

#25276 was marked as a duplicate.

comment:27 adamsilverstein4 months ago

  • Cc adamsilverstein@… added
Note: See TracTickets for help on using tickets.