WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 8 months ago

Last modified 7 months ago

#20496 closed enhancement (fixed)

Previews should redirect to the permalink if the post has been published

Reported by: evansolomon Owned by: wonderboymusic
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch commit
Focuses: Cc:

Description

If a post is saved as a draft and you visit the preview URL, even after the post is published, you will stay on the preview view (with preview=true in the URL). This is confusing since you're not actually previewing anything, you're viewing a public post.

Attachments (4)

published-post-preview-canonical-redirect.patch (1.6 KB) - added by amit 2 years ago.
preview-patch.diff (1.5 KB) - added by joostdevalk 17 months ago.
Improved patch
20496.diff (656 bytes) - added by JustinSainton 8 months ago.
docs typo
20496.2.diff (536 bytes) - added by SergeyBiryukov 8 months ago.

Download all attachments as: .zip

Change History (32)

comment:1 follow-up: @jane3 years ago

  • Keywords reporter-feedback added

Are you talking about someone clicking on the Preview Changes button to get to the Preview URL, or refreshing/copy/pasting the URL? I think I disagree with the suggestion in both cases, but want to be sure I understand which scenario you're talking about before posting counter opinion.

comment:2 @nacin3 years ago

If ?preview=true and ! $_POST (does it work via $_GET?), we can probably shave off the query var in canonical. Makes sense, as that URL shouldn't be distributed.

comment:3 in reply to: ↑ 1 @evansolomon3 years ago

Replying to jane:

Are you talking about someone clicking on the Preview Changes button to get to the Preview URL, or refreshing/copy/pasting the URL?

Neither, at least not specifically. Here is a way to get to the point I'm talking about:

1) Write a post and save it as a draft
2) Send a link to the post preview to someone else (i.e. "What do you think of this post? [link]")
3) Get impatient waiting for your friend to reply and publish your post
4) Now your friend clicks on your "preview" link

At step 4 there's nothing being previewed, just a confusing URL that looks like it is. Your friend is viewing a public post that would work without the "preview=true" in the URL, whether or not they have any capabilities on the blog. If the "preview=true" were removed, it would run through redirect_canonical() and send the user to the post permalink.

Nacin's interpretation is exactly what I intended.

comment:4 @evansolomon3 years ago

  • Keywords reporter-feedback removed

comment:5 follow-up: @Ipstenu3 years ago

This just popped up in the forums. Here's a fun example.

http://ipstenu.org/?p=1917&preview=true

The page is published, but it doesn't redirect.

Worse another person is having a problem where a NON published page is showing something.

http://wordpress.org/support/topic/redirecting-page-preview

And it's showing up in Google.

comment:6 in reply to: ↑ 5 @SergeyBiryukov3 years ago

Replying to Ipstenu:

Worse another person is having a problem where a NON published page is showing something.

Looking at the thread, this doesn't seem to be the case. The page appears to be published.

comment:7 @ipstenu3 years ago

It wasn't showing published at the time. When I hit the page sans preview it was a 404.

comment:8 @nacin3 years ago

&preview=true only works if you are a logged-in user with permission to edit that post. If you are not logged in, it is impossible (without a plugin deliberately or accidentally making changes to a deep area of WP_Query) to view a non-published post.

There's no information disclosure bug here. At most, that's a misunderstanding. The issue is purely canonically redirecting the link once the post is published, in case it is shared.

comment:9 @azaozz3 years ago

  • Keywords close added; needs-patch removed

Previews work for both un-published and published posts. In the first case the post is shown on the front end.

In the second case the post_content is taken from the editor and shown on the front end, so when previewing a published post, you are looking at any edits/changes made since the post was published. That's why the button's name changes to Preview Changes.

So, to answer directly to the concern in the ticket:

"This is confusing since you're not actually previewing anything, you're viewing a public post."

No, you are not viewing a public post, you are previewing changes made since the post was published.

comment:10 @nacin3 years ago

  • Keywords needs-patch added; close removed

No, you are not viewing a public post, you are previewing changes made since the post was published.

Not quite — that's only if the link has a preview_id and preview_nonce.

A draft post that is ?p=1234&preview=true does not trigger a preview once the post is published. It simply loads the post. The preview query variable is ignored. This ticket is about removing the preview query variable in this case and doing a canonical redirect to wherever ?p=1234 should go.

comment:11 @azaozz3 years ago

Ah, I see. So this would only cover the cases where a preview link was opened by another user, then the original author has published the post and the other user refreshes the preview page instead of going to the permalink for the published page. Seems like quite rare edge case.

Version 0, edited 3 years ago by azaozz (next)

comment:13 @helen2 years ago

#23344 was marked as a duplicate.

comment:14 @mintindeed2 years ago

  • Cc gabriel.koen@… added

comment:15 @amit2 years ago

  • Cc coolamit@… added

comment:16 @amit2 years ago

  • Keywords has-patch added; needs-patch removed

added patch

comment:17 @nacin2 years ago

  • Milestone changed from Awaiting Review to 3.6

comment:18 @ryan22 months ago

  • Milestone changed from 3.6 to Future Release

@joostdevalk17 months ago

Improved patch

comment:19 @joostdevalk17 months ago

  • Keywords commit added

Looked at this patch with @sergeybiryukov and @nacin, seems fine now.

comment:20 @juliobox11 months ago

Hello, patch tested seems OK for me too, i was wondering if this ticket exists : yes ;)
Thanks in advance for the future acceptation of this patch.

comment:21 @wonderboymusic8 months ago

  • Milestone changed from Future Release to 4.0

comment:22 @wonderboymusic8 months ago

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

In 28874:

Perform a canonical redirect for posts that are published but are visited at a ?p=123&preview=true URL.

Props amit, joostdevalk.
Fixes #20496.

@JustinSainton8 months ago

docs typo

comment:23 @JustinSainton8 months ago

Minor typo in inline docs

comment:24 @SergeyBiryukov8 months ago

In 28878:

Fix typo in a comment.

props JustinSainton.
see #20496.

comment:25 @SergeyBiryukov8 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[28874] broke four unit tests:

There were 4 failures:

1) Tests_Canonical::test with data set #10 ('/?category_name=cat-a,cat-b', array('/?category_name=cat-a,cat-b', array('cat-a,cat-b')))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/?category_name=cat-a,cat-b'
+'/?category_name=cat-a%2Ccat-b'

S:\home\wordpress\develop\tests\phpunit\tests\canonical.php:133
S:\usr\local\php5\phpunit:46

2) Tests_Canonical_HTTPS::test with data set #10 ('/?category_name=cat-a,cat-b', array('/?category_name=cat-a,cat-b', array('cat-a,cat-b')))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/?category_name=cat-a,cat-b'
+'/?category_name=cat-a%2Ccat-b'

S:\home\wordpress\develop\tests\phpunit\tests\canonical.php:133
S:\usr\local\php5\phpunit:46

3) Tests_Canonical_NoRewrite::test with data set #3 ('/?p=358 ', array('/?p=358', array('358')))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/?p=358'
+'/?p=358+'

S:\home\wordpress\develop\tests\phpunit\tests\canonical.php:133
S:\usr\local\php5\phpunit:46

4) Tests_Canonical_NoRewrite::test with data set #4 ('/?p=358%20', array('/?p=358', array('358')))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/?p=358'
+'/?p=358+'

S:\home\wordpress\develop\tests\phpunit\tests\canonical.php:133
S:\usr\local\php5\phpunit:46

Caused by remove_query_arg(). The first two seem minor, but the other two look like an issue.

@SergeyBiryukov8 months ago

comment:26 @SergeyBiryukov8 months ago

20496.2.diff fixes the test issues.

The ! is_preview() check is redundant, we already check for that earlier. See line 52 in [28874].

comment:27 @SergeyBiryukov8 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 28928:

Replace a redundant ! is_preview() check with a more appropriate one.

fixes #20496.

comment:28 @SergeyBiryukov7 months ago

#29091 was marked as a duplicate.

Note: See TracTickets for help on using tickets.