Skip to content

Fixes published-post autosave-previews in calypso#6530

Merged
dereksmart merged 2 commits intomasterfrom
fix/published-autosave-previews-calypso
Feb 28, 2017
Merged

Fixes published-post autosave-previews in calypso#6530
dereksmart merged 2 commits intomasterfrom
fix/published-autosave-previews-calypso

Conversation

@rralian
Copy link
Copy Markdown
Contributor

@rralian rralian commented Feb 28, 2017

The wpcom endpoint cannot predict whether the user is logged in or not on the remote site. So it cannot pass the appropriate nonce that gets verified in /wp-includes/revision.php. This patch detects the frame-nonce parameter for a caypso preview and falls back to the existing frame-nonce validation when it exists.

Fixes Automattic/wp-calypso#5607

Testing instructions:

  1. Confirm the issue: Without this patch, go to wordpress.com, select a jetpack site and go to /posts/:site, then start to "edit" a published post (not a draft!) in Calypso. Now start typing, and without saving click the "view" button in the left sidebar. It will open a new tab that will resolve to a page that says you don't have permission to view drafts, whether you are logged-in to the remote site or not.
  2. Confirm the fix: Apply this patch and follow the above instructions. The new tab will resolve to a working post preview. This preview URL with the frame-nonce will work whether you are logged-in to the remote site or not.
  3. Confirm no regressions: With this patch follow the above instructions with an unpublished draft (the button will say preview instead of view). Confirm previews are working correctly. Now try the same with a published post and a draft from wp-admin.

Caveat

FWIW I am seeing an error in the preview page, but it seems unrelated. Still worth confirming.

preview page error

/cc @ebinnion

@rralian
Fixes published-post autosave-previews in calypso
3fd99f8 
The wpcom endpoint cannot predict whether the user is logged in or not on the remote site. So it cannot pass the appropriate nonce that gets verified in `/wp-includes/revision.php`. This patch detects the frame-nonce parameter for a caypso preview falls back to the existing frame-nonce validation when it exists.
@rralian rralian added [Status] Needs Review This PR is ready for review. Bug When a feature is broken and / or not performing as intended [Pri] High labels Feb 28, 2017
@retrofox
Copy link
Copy Markdown
Contributor

retrofox commented Feb 28, 2017 •
edited by zinigor
Loading

Testing tasks:

  • Confirm the issue
  • Confirm the fix
  • Confirm no regressions

@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 28, 2017
zinigor
zinigor approved these changes Feb 28, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, tested on both drafts and published posts. I'm assuming that it's expected behaviour that when you edit a post and click View without saving, you don't see your changes on the Jetpack site. To see them you'd need to Update, right?

dereksmart
dereksmart reviewed Feb 28, 2017
View reviewed changes
class.frame-nonce-preview.php Outdated

/**
* Handle validation for autosave preview request
*
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good and tests well. Could you please just fix the wonky indentation in this block? Thanks!

Copy link
Copy Markdown
Contributor Author

@rralian rralian Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks... I don't have a proper jetpack dev setup so I was mangling this with nano. Didn't notice the spacing.

@retrofox
Copy link
Copy Markdown
Contributor

retrofox commented Feb 28, 2017

LGTM

dereksmart
dereksmart reviewed Feb 28, 2017
View reviewed changes
class.frame-nonce-preview.php Outdated
*/
public function handle_autosave_nonce_validation() {
if ( ! $this->is_frame_nonce_valid() ) {
wp_die( __( 'Sorry, you are not allowed to preview drafts.' ) );
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart Feb 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing 'jetpack' textdomain

Copy link
Copy Markdown
Contributor Author

@rralian rralian Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, done.

@dereksmart
Copy link
Copy Markdown
Contributor

dereksmart commented Feb 28, 2017

FWIW I am seeing an error in the preview page, but it seems unrelated.

Indeed unrelated

@rralian
Copy link
Copy Markdown
Contributor Author

rralian commented Feb 28, 2017

Looks good, tested on both drafts and published posts. I'm assuming that it's expected behaviour that when you edit a post and click View without saving, you don't see your changes on the Jetpack site. To see them you'd need to Update, right?

Yes, that's correct. It's basically doing a "preview" of your changes, but without updating the published post until you're happy with it and decide to "update".

@dereksmart dereksmart merged commit facdba6 into master Feb 28, 2017
@dereksmart dereksmart deleted the fix/published-autosave-previews-calypso branch February 28, 2017 22:19
@dereksmart dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 28, 2017
jeherve added a commit that referenced this pull request Feb 28, 2017
@jeherve
Changelog: add #6530
1d567b7
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
@jeherve @dereksmart
Jetpack 4.7: Changelog (#6331)
a9adf86 
* Changelog: update stable tag and move changelog to changelog.txt

Also remove old releases from readme.txt to keep the changelog tab short.

* Changelog: add #5883

Also update the filter's docblock to match new version.

* Changelog: add #5938

* Changelog: add #6298

* Changelog: add #3405

* Changelog: add #5941

* Changelog: add #6239

* Changelog: add #6281

* Changelog: add #6303

* Changelog: add #6018

* Changelog: add #6300

* Changelog: add #6296

* Changelog: add #6130

* Changelog: add #6292

* Readme: remove extra "on".

* Changelog: add #6307

* Changelog: add #3297

* Changelog: add #6275

* Changelog: add #6321

* Changelog: add #6297

* Readme: update the support forum link anchor.

Anchor changed when WordPress.org forums were updated to bbPress 2

* Readme: update list of a12s, it wasn't up to date anymore!

* Changelog: add #6338

* Changelog: add #6337

* Changelog: add #6335

* Changelog: add #6333

* Testing List: first version of the 4.7 testing list.

* Changelog: add #6332

* Changelog: add #6325

* Changelog: add #6326

* Changelog: add #6339

* Changelog: add #6342

* Changelog: add #6343

* Changelog: add #6346

* Changelog: add #6347

* Changelog: add #6279

* Changelog: add #6306

* Changelog: add #6312

* Changelog: add #6316

* Changelog: add #6171

* Changelog: add #6317

* Changelog: add #6246

* Changelog: add #6263

* Changelog: add #4220

* Changelog: add #5888

* Changelog: add #3406

* Changelog: add #3637

* Changelog: add #6320

* Changelog: add #5992

* Changelog: add #6322

* Changelog: add #6324

* Changelog: add #6352

* Changelog: add #6355

* Changelog: add #6360

* Changelog: add #6362

* Changelog: add #6369, #6382

* Changelog: add #6370

* Changelog: add #6375

* Changelog: add #6383

* Changelog: add #6384

* Changelog: add #6386

* Changelog: add #6395

* Changelog: add #6403

* Changelog: add #6406

* Changelog: add #6418

* Changelog: add #6419

* Changelog: add #6434

* Changelog: add #6446

* Changelog: add #6006

* Changelog: add #6096

* Changelog: add #6399

* Changelog: fix typo.

@see #6331 (comment)

* Changelog: add #6440

* Changelog: add #6443

* Changelog: add #6445

* Changelog: add #6463

* Changelog: add #6468

* Changelog: add #6471

* Changelog: add #6474

* Changelog: add #6480

* Changelog: add #6497

* Changelog: add #6499

* Changelog: add #6514

* Changelog: add #6267

* Changelog: add #5940

* Changelog: add #6492

* Changelog: add #5281

* Changelog: add #6327

* Changelog: add #6451

* Changelog: add #6525

* Changelog: add #6530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dereksmart dereksmart dereksmart left review comments

+1 more reviewer

@zinigor zinigor zinigor approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug When a feature is broken and / or not performing as intended [Pri] High

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Editor: previewing changing posts on Jetpack sites broken

5 participants

@rralian @retrofox @dereksmart @zinigor @jeherve