Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Restore unification of multi-page post content in Reader mode #4547

Merged
merged 3 commits into from
Apr 9, 2020
Merged

Restore unification of multi-page post content in Reader mode #4547

merged 3 commits into from
Apr 9, 2020

Conversation

westonruter
Copy link
Member

Summary

Originally reported on a support forum topic:

Since the last update to your AMP plugin (which we’ve been using with the Zox News WordPress theme for over a year), we’ve encountered this issue. We use <!--nextpage--> to paginate our big features on desktop/mobile (like this one). Your plugin has always ignored this code and presented those features as one big page. Now, though, our AMP pages are killing all content after it hits the <!--nextpage--> directive, meaning that our readers aren’t seeing our full content. Click here to see how the AMP version of the list above is ending at number 91. We want to avoid having to un-paginate our features in order to correct this issue. Thanks.

The regression was introduced in #3781 via this change: https://github.com/ampproject/amp-wp/pull/3781/files#r405130526

The switch to get_the_content() caused pagination to be introduced which was not present before, when post_content was directly passed into the_content filters.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added Bug Something isn't working Reader Mode labels Apr 7, 2020
@westonruter westonruter added this to the v1.5.3 milestone Apr 7, 2020
@googlebot googlebot added the cla: yes Signed the Google CLA label Apr 7, 2020
westonruter
westonruter commented Apr 7, 2020
View reviewed changes
includes/templates/class-amp-post-template.php
@@ -342,8 +342,14 @@ private function build_post_comments_data() {
* Build post content.
*/
private function build_post_content() {
/** This filter is documented in wp-includes/post-template.php */
$content = apply_filters( 'the_content', get_the_content( null, false, $this->post ) );
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this did not need the post_password_required() check before because get_the_content() has that logic inside of it.

@westonruter
Copy link
Member Author

Workaround plugin which implements this while waiting for 1.5.3: https://gist.github.com/westonruter/559c7a9cbb9460d1099c3d5d9bfa5ad1

@westonruter westonruter requested a review from pierlon April 7, 2020 22:05
@westonruter westonruter mentioned this pull request Apr 7, 2020
Merged
9 tasks
@westonruter @schlessera
Combine comments
a16496e 
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@westonruter westonruter merged commit 1868ea2 into develop Apr 9, 2020
@westonruter westonruter deleted the fix/reader-mode-multipage-pagination branch April 9, 2020 21:16
westonruter added a commit that referenced this pull request Apr 9, 2020
@westonruter @schlessera
Restore unification of multi-page post content in Reader mode (#4547)
73db741 
* Restore unification of multi-page post content in Reader mode

* Combine comments

Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>

* Fix Squiz.WhiteSpace.SuperfluousWhitespace.EndLine

Co-authored-by: Alain Schlesser <alain.schlesser@gmail.com>
westonruter added a commit that referenced this pull request Apr 10, 2020
@westonruter
Merge branch 'develop' of github.com:ampproject/amp-wp into add-css-c…
daa92ca 
…aching-reenable-button

* 'develop' of github.com:ampproject/amp-wp:
  Restore unification of multi-page post content in Reader mode (#4547)
  Prevent styles from being removed when in Customizer preview with Standard mode (#4553)
  Omit Jetpack from being activated during PHPUnit test runs
  Use title case for Paired Browsing link in edit post screen (#4540)
  Ensure that validation query vars persist through redirects (#4544)
  Update dependency babel-jest to v25.2.6 (#4510)
  Update dependency css-loader to v3.5.0 (#4537)
  Update dependency autoprefixer to v9.7.6 (#4539)
  Add requirements to plugin file header (#4543)
  Force status code of validation responses to be 200 (#4533)
  Update optimizer test specs (#4527)
  Bump stable tag to 1.5.2
  Cache response status and headers when fetching external stylesheets (#4509)
  Fix securing multi-line mustache templates (#4521)
  Add CSS monitoring time series to Site Health debugging info (#4519)
  Update hostname used for WordPress TV embeds to fix external HTTP requests (#4524)
  Mock Imgur embed tests
  Mock Facebook embed tests
  Standardize file and class names for embed handlers
westonruter added a commit that referenced this pull request Apr 15, 2020
@westonruter
Merge tag '1.5.3'
4e4195e 
* tag '1.5.3':
  Bump 1.5.3
  Bump version to 1.5.3-RC1
  Fix handling of Mustache templates (#4583)
  Stub request based on test scenario (#4588)
  Update tests after block-library/style.css changes in Gutenberg 7.9 (#4579)
  Restrict doing plugin upgrade routine when not in admin (#4538)
  Add new accessibility sanitizer (#4535)
  Fix unit tests (#4564)
  Add button into Site Health to reenable CSS transient caching (#4522)
  Restore unification of multi-page post content in Reader mode (#4547)
  Prevent styles from being removed when in Customizer preview with Standard mode (#4553)
  Omit Jetpack from being activated during PHPUnit test runs (#4474)
  Mock Facebook embed tests (#4474)
  Mock Imgur embed tests (#4474)
  Use title case for Paired Browsing link in edit post screen (#4540)
  Ensure that validation query vars persist through redirects (#4544)
  Add requirements to plugin file header (#4543)
  Force status code of validation responses to be 200 (#4533)
  Update optimizer test specs (#4527)
  Bump 1.5.3-alpha
@westonruter westonruter mentioned this pull request May 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierlon pierlon pierlon approved these changes

@schlessera schlessera schlessera approved these changes

Assignees
No one assigned
Labels
Bug Something isn't working cla: yes Signed the Google CLA Reader Mode
Projects
None yet
Milestone
v1.5.3
Development

Successfully merging this pull request may close these issues.
4 participants
@westonruter @schlessera @pierlon @googlebot