Markup prepended via the_content filter causes amp-story to be removed from page

honeyrbhandari commented 4 years ago

Hi,

Please help me. The amp stories are not working in my WP site. Here is one of the story, I created - https://wisdom2shine.com/stories/listen-to-your-true-calling/ It goes into a blank screen. It was working fine earlier. However, now it fails to load and shows the following error message.

Even when I try to edit it, it does not allow me to do so because of the following validation error mesage:

Please help as I need to create content in the form of stories and I am struggling hard to get it resolved :(...

Will look forward to a resolution. Best Regards, Honey

westonruter commented 4 years ago

The rejected validation error for the script is likely the problem. Switch that to accepted.

We need to prevent this endless redirect from happening.

I'm confused why a script is getting added to the Story template in the first place, however. Please expand the details for both of those validation errors.

westonruter commented 4 years ago

The error for amp-story is also going to be a problem, because it will show the story as blank when removed. So yeah, pleas click details for each and share screenshots.

honeyrbhandari commented 4 years ago

Hi,

Thanks a lot for your prompt response! Here are the screenshots:

Best Regards, Honey

westonruter commented 4 years ago

@honeyrbhandari Do you know which plugin is responsible for adding the WebFontConfig? In any case, that validation error should be marked as “Accepted” because the AMP Stories editor handles the fonts already, and custom JS is not allowed.

The second validation error regarding amp-story is more confusing. Can you share a screenshot of the editor for that story?

Please also do this:

Mark that amp-story error as “rejected”.
Access the URL of the story but add ?amp_validate to the URL. This will prevent that endless redirect bug.
Share the HTML source of the the page (via browser “View Page Source”). You can paste that into a Gist.

westonruter commented 4 years ago

Here's also a build of the plugin which fixes the redirect issue: amp.zip (1.3-RC1-20190923T180453Z-6a4424f0). See #3329.

Still, it's not clear to me why the amp-story element is invalid on your site.

honeyrbhandari commented 4 years ago

Sure, Thanks a lot for helping me out. I did the following tasks and created one brand new story and thankfully, it started showing up. Here are the steps i have taken:

Rejected the error
Uninstalled custom fonts plugin - adobe fonts. Now, when I m creating a new story, it is showing the story pages; However, one old Story is still getting into the white screen of death and here is its source code- [white screen error - AMP story.zip](https://github.com/ampproject/amp-wp/files/3643704/white.screen.error.-.AMP.story.zip
Will try creating a new story after installing the 1.3 version that you have shared. Will do it maybe tomorrow and will keep you posted. I want to thank you again. I have been struggling hard from last many days and was not getting any solution You have made stories to at least show-up.

Just one query: Should I always set errors to rejected/reject in order for Stories to work?? Also, To test stories, I tried creating one story in MakeStories.io and it was working perfectly fine in their server. However, when I am trying to publish it on my website, it says WP does not support stories/MakeStories.....Please help with this too.

Thanks Much! God Bless You Bountiful! Best Regards, Honey

westonruter commented 4 years ago

In general, validation errors should always be marked as accepted. Only in rare situations would a validation error be marked as rejected. It's main purpose is to help with debugging, so you can clearly see what functionality is impacted by the sanitized element/attribute. Such as in this case here, please mark as Rejected the amp-story validation error and then share the page source again. A rejected validation error means that the page will not be served as valid AMP, meaning it won't be able to take advantage of the AMP Cache or to be embedded in other products, like Google Search and Google Discover.

From what you shared, I can see the font issue is due to Jetpack as it's coming from the Jetpack_Fonts::maybe_render_fonts() method. But I'm seeing that inside the Jetpack plugin, so apparently it is coming from another plugin or perhaps the theme.

I don't know about MakeStories.io so I can't really advise on that part. You should either use the AMP Stories editor in this plugin or the MakeStories.io editor, not both.

honeyrbhandari commented 4 years ago

testing-1-3.zip Hi, Created a new one using AMP 1.3 --> https://wisdom2shine.com/stories/testing-1-3/ and rejected its errors. But sadly, it went into the white screen of death. Strangely, it worked when I check a few minutes back with just 2 story pages.

Thanks, Best Regards, Honey

westonruter commented 4 years ago

What theme/plugins do you have active? You could share information from Site Health in the WP Admin.

honeyrbhandari commented 4 years ago

I m consistently using AMP stories in WP and thats what I intend to use. MakeStories was just a testing thing on a third-party option. I tried it to explore if the issue that I m seeing is across different platforms or it's just with WP. Anyways. I m back to square one... with this new one also behaving the same way as the old one and not showing up.

I wish this gets solved once and for all. Awaiting for the same! :)

Thanks, Best Regards, Honey

honeyrbhandari commented 4 years ago

Sure... Here it is...

wp-core

version: 5.2.3 site_language: en_US user_language: en_US permalink: /%postname%/ https_status: true user_registration: 1 default_comment_status: open multisite: false user_count: 5 dotorg_communication: true

wp-paths-sizes

wordpress_path: /wordpress/core/5.2.3 wordpress_size: 40.64 MB (42611777 bytes) uploads_path: /srv/htdocs/wp-content/uploads uploads_size: 1.18 GB (1266840162 bytes) themes_path: /srv/htdocs/wp-content/themes themes_size: 29.35 MB (30771593 bytes) plugins_path: /srv/htdocs/wp-content/plugins plugins_size: 81.95 MB (85935604 bytes) database_size: 10.61 MB (11124736 bytes) total_size: 1.34 GB (1437283872 bytes)

wp-dropins (2)

advanced-cache.php: true object-cache.php: true

wp-active-theme

name: Canuck version: 1.2.3 author: Kevin Archibald author_website: https://kevinsspace.ca/ parent_theme: none theme_features: custom-header, post-formats, custom-background, automatic-feed-links, editor-style, post-thumbnails, html5, title-tag, custom-logo, align-wide, amp, widgets, menus theme_path: /srv/htdocs/wp-content/themes/canuck

wp-themes (15)

Baskerville 2: version: 2.0.1-wpcom, author: Anders Norén Baskerville: version: 1.35, author: Anders Norén Dyad 2: version: 2.0.8-wpcom, author: Automattic Natural: version: 1.0, author: Organic Themes Photo Blog: version: 1.1.1-wpcom, author: Automattic Poly: version: 1.0, author: Automattic Rosalie: version: 1.0.3, author: SiloCreativo Sidebar: version: 1.0.6, author: MetricThemes Silvio: version: 1.0.6, author: SiloCreativo Storytime: version: 1.0.4, author: Rough Pixels The Writer: version: 1.0-wpcom, author: Obox Themes Twenty Fifteen: version: 2.3-wpcom, author: the WordPress team Twenty Nineteen: version: 1.3-wpcom, author: the WordPress team VideoStories: version: 2.0.3, author: Liton Arefin Vision: version: 1.7, author: Pro Theme Design

wp-mu-plugins (1)

WP.com Site Helper: author: (undefined), version: (undefined)

wp-plugins-active (10)

Akismet Anti-Spam: version: 4.1.2, author: Automattic AMP: version: 1.3-RC1-20190923T180453Z-6a4424f0, author: AMP Project Contributors Classic Editor: version: 1.5, author: WordPress Contributors Display PHP Version: version: 1.5, author: David Gwyer Full Site Editing: version: 0.7, author: Automattic Google XML Sitemaps: version: 4.1.0, author: Arne Brachhold Gutenberg: version: 6.5.0, author: Gutenberg Team Image Regenerate & Select Crop: version: 5.01, author: Iulia Cazan Jetpack by WordPress.com: version: 7.7.2, author: Automattic Reading Time WP: version: 2.0.5, author: Jason Yingling

wp-plugins-inactive (4)

Ads for WP - Advanced Ads & Adsense Solution for WP & AMP: version: 1.9.6, author: Magazine3 AMP: version: 1.2.2, author: AMP Project Contributors Design Experiments: version: 1.3, author: The WordPress.org Design Team Super Socializer: version: 7.12.34, author: Team Heateor

wp-media

image_editor: WP_Image_Editor_GD imagick_module_version: Not available imagemagick_version: Not available gd_version: bundled (2.1.0 compatible) ghostscript_version: not available

wp-server

server_architecture: Linux 4.9.0-9-amd64 x86_64 httpd_software: nginx php_version: 7.2.22 64bit php_sapi: fpm-fcgi max_input_variables: 6144 time_limit: 1200 memory_limit: 256M max_input_time: 1200 upload_max_size: 2047M php_post_max_size: 2047M curl_version: 7.65.3 OpenSSL/1.1.0k suhosin: false imagick_availability: false

wp-database

extension: mysqli server_version: 5.5.5-10.3.17-MariaDB-log client_version: mysqlnd 5.0.12-dev - 20150407 - $Id: 3591daad22de08524295e1bd073aceeff11e6579 $

wp-constants

WP_HOME: https://wisdom2shine.com WP_SITEURL: https://wisdom2shine.com WP_MAX_MEMORY_LIMIT: 256M WP_DEBUG: false WP_DEBUG_DISPLAY: false WP_DEBUG_LOG: false SCRIPT_DEBUG: false WP_CACHE: true CONCATENATE_SCRIPTS: undefined COMPRESS_SCRIPTS: undefined COMPRESS_CSS: undefined WP_LOCAL_DEV: undefined

wp-filesystem

wordpress: not writable wp-content: writable uploads: writable plugins: writable themes: writable mu-plugins: writable

westonruter commented 4 years ago

Can you share a screenshot of the Jetpack modules you have active?

westonruter commented 4 years ago

It seems what is happening is there is some the_content filter which is corrupting the AMP Story elements. For example, a simple Story with one single page and one single Text block should have the_content() output something like this:

<amp-story-page style="background-color:#ffffff" id="291751cd-9d1b-4570-8ed9-fd9b2ab43e4f" class="wp-block-amp-amp-story-page">
    <amp-story-grid-layer template="fill"></amp-story-grid-layer>
    <amp-story-grid-layer template="vertical">
        <div class="amp-story-block-wrapper" style="position:absolute;top:48.46%;left:7.93%;width:76.22%;height:10.85%;">
            <h1 style="display:flex" class="wp-block-amp-amp-story-text">
                <amp-fit-text layout="flex-item" class="amp-text-content">Hello, World!</amp-fit-text>
            </h1>
        </div>
    </amp-story-grid-layer>
</amp-story-page>

Some plugin seems to be manipulating this so that the AMP Story elements are dropped. This then causes the amp-story to be invalid.

westonruter commented 4 years ago

I found the problem. It's the Reading Time WP plugin.

It is adding this markup to the beginning of `the_content:

<span class="rt-reading-time" style="display: block;"><span class="rt-label">Reading Time: </span> <span class="rt-time">1</span> <span class="rt-label rt-postfix">minute</span></span>

So the quick fix is simply to turn off the Reading Time on Stories:

The deeper fix is for the plugin's Story sanitizer to be more aggressive about removing such invalid elements, in order to prevent invaliding the whole amp-story.

westonruter commented 4 years ago

I've created a pull request to fix the underlying problem in #3336.

westonruter commented 4 years ago

Steps to reproduce:

Activate Reading Time WP plugin.
Ensure “Display on Stories” option is enabled in Reading Time WP settings.
Create a new Story.
Before fix, a validation error appears with amp-story being removed; the story is a white page. After fix in #3336, a validation error appears for a span and the story is preserved when viewing.
Also, with the fix in #3329, you can freely Reject the validation error for the span and no infinite redirect will occur.

honeyrbhandari commented 4 years ago

Yippie!!!!!! It worked!!!

@westonruter Did someone tell u lately that you are an Angel!!!!

Appreciate your endless support... u did not leave it till it was solved.

Blessings!!! Blessings!!!! Blessings!!! to you... @westonruter - Be ultra-successful in all your endeavors!!!! Respect! Respect! n overflowing respect n happiness!!!!!!! :) :)

God bless you bountiful! @westonruter Best Regards, Honey

swissspidy commented 4 years ago

Also, with the fix in #3329, you can freely Reject the validation error for the span and no infinite redirect will occur.

@westonruter When I reject the validation error for the span, a second one pops up for amp-story and the story is a white page. Is that expected?

Screenshot 2019-09-27 at 10 18 33

westonruter commented 4 years ago

When I reject the validation error for the span, a second one pops up for amp-story and the story is a white page. Is that expected?

Humm. I suppose that is expected, indeed. But it doesn't seem ideal. I'm not sure what should be done in this case.

westonruter commented 4 years ago

Re-opening to think this through some more.

westonruter commented 4 years ago

Actually, I'm going to close this. I've created #3393 to follow up.

ampproject / amp-wp