WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 7 weeks ago

Last modified 7 weeks ago

#17157 closed defect (bug) (fixed)

Cannot preview changes to published multi-page posts

Reported by: akoyfman Owned by: johnbillion
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.0.4
Component: Posts, Post Types Keywords: has-patch
Focuses: template Cc:

Description

Use Case: After publishing a post, an editor needs to make changes/update the post. Editor would like to preview changes before publishing. (Permalinks are turned on in this case)

To Reproduce:

  1. Write a post. Make it paged by putting <!--nextpage--> somewhere in the post.
  2. Publish the post
  3. Make changes to the post, somewhere on page 2
  4. Click "Preview Changes" button

Browser will open a new page for the post: http://[domain]/yyyy/mm/dd/[post title]/?preview=true&preview_id=126507&preview_nonce=f0a2ecd9ae

  1. Next page link will point to http://[domain]/yyyy/mm/dd/[post title]/ (Note: that preview, preview_id and preview_nonce are missing from the url)

Suggested fix is attached.

Attachments (3)

post-template.php (37.9 KB) - added by akoyfman 3 years ago.
17157.diff (758 bytes) - added by solarissmoke 3 years ago.
17157.2.diff (972 bytes) - added by chrisscott 2 years ago.

Download all attachments as: .zip

Change History (14)

akoyfman3 years ago

comment:1 follow-up: kawauso3 years ago

Please provide a patch against trunk from SVN. Info on how to create patches is available on the Trac homepage.

comment:2 in reply to: ↑ 1 akoyfman3 years ago

Replying to kawauso:

Please provide a patch against trunk from SVN. Info on how to create patches is available on the Trac homepage.

Not sure if this was addressed to me - I am not an active developer. I rather hope that someone who knows the code can take a look at the bug. The suggested fix was just meant as a help to show where the problem is occurring.

solarissmoke3 years ago

comment:3 solarissmoke3 years ago

  • Keywords has-patch added

comment:4 akoyfman2 years ago

Fixes the problem - thank you. (If someone can put this into the main branch, it would be much appreciated)

chrisscott2 years ago

comment:5 chrisscott2 years ago

Refreshed patch against r20755 trunk. Updated to only add preview_id and preview_nonce query vars when not a draft to be consistent with standard preview functionality.

comment:6 nacin3 months ago

  • Component changed from General to Template
  • Milestone changed from Awaiting Review to 3.9

Patch seems good.

comment:7 nacin3 months ago

  • Component changed from Template to Posts, Post Types
  • Focuses template added

comment:8 johnbillion7 weeks ago

I had thought that this patch introduced a very subtle permissions bug by always generating a valid nonce and adding it to subsequent page URLs, but the protection comes in WP_Query::get_posts().

Good to go.

comment:9 johnbillion7 weeks ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 27333:

Enable previews for all pages of multi-page posts. Fixes #17157. Props solarissmoke, chrisscott

comment:10 johnbillion7 weeks ago

In 27334:

Pass along preview query args only if they are already present. Avoids sloppily appending a preview nonce when there should not be one. See #17157.

comment:11 nacin7 weeks ago

Some stuff in IRC didn't get logged due to some issues with the bot:

6:12	nacin	johnbillion: try adding ?preview=true to the end of a URL when logged out
6:13	johnbillion	nacin: Bah. Interestingly that's what I mentioned in the comment prior to the commit. Note that the preview doesn't actually get displayed though because the user doesn't have permission to edit the post.
6:14	johnbillion	I had thought of passing along the current $_GET['preview_nonce'] instead - think that's a better approach?
6:14	nacin	Yeah, and technically leaking this nonce isn't an issue that I can tell. But it's sloppy. Trying to figure out how to fix it.
6:15	nacin	perhaps setting is_preview => true in WP_Query should immediately validate the nonce. I'm honestly not sure. it's fairly old code.
6:16	nacin	passing along $_GET seems like a good immediate fix.
6:16	nacin	perhaps the conditional should be if ( 'draft' !== $status && isset( $_GET['preview_nonce'] ) )
6:16	nacin	mentioning #17157 so this makes it back to the ticket.
6:16	trac-bot	nacin: http://core.trac.wordpress.org/ticket/17157 3.9, akoyfman->johnbillion, closed, Cannot preview changes to published multi-page posts
6:16	johnbillion	Yeah and I'l check for $_GET['preview_id'] too
6:17	nacin	I just meant as a quick check; if the other isn't set and it notices, it's not a huge deal.
6:17	nacin	but yeah, isset( $_GET['preview_nonce'], $_GET['preview_id'] ) is fine.
6:18	nacin	_show_post_preview() should really happen inside WP_Query::parse_query() with an opportunity to set is_preview to false.
Note: See TracTickets for help on using tickets.