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

Components for amp-story and amp-story-page are removed if invalid children are rejected from sanitization #3393

Closed
westonruter opened this issue Oct 1, 2019 · 1 comment
Closed

Components for amp-story and amp-story-page are removed if invalid children are rejected from sanitization #3393

westonruter opened this issue Oct 1, 2019 · 1 comment
Labels
Groomed P2 Low priority Sanitizers wontfix WS:Perf Work stream for Metrics, Performance and Optimizer

Comments

@westonruter
Copy link
Member

As noted by @swissspidy in #3321 (comment):

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

This is not ideal. The amp-story and amp-story-page should never be removed due to having invalid children.

We need to revisit #3336, perhaps adapting #2926 to special-case the amp-story and amp-story-page components.
@westonruter westonruter added this to the v1.3.1 milestone Oct 1, 2019
@westonruter westonruter self-assigned this Oct 1, 2019
@westonruter westonruter mentioned this issue Oct 1, 2019
Closed
@swissspidy swissspidy modified the milestones: v1.3.1, v1.4 Oct 17, 2019
@kmyram kmyram removed this from the v1.4 milestone Oct 17, 2019
@westonruter westonruter removed their assignment Oct 24, 2019
@jauyong jauyong added the P2 Low priority label Nov 12, 2019
@westonruter westonruter reopened this Nov 26, 2019
@kmyram kmyram added the WS:Perf Work stream for Metrics, Performance and Optimizer label Aug 5, 2020
@westonruter westonruter mentioned this issue Aug 5, 2020
Closed
@westonruter westonruter mentioned this issue Sep 16, 2020
Merged
@westonruter
Copy link
Member Author

I'm going to close this for now since the issue is no longer relevant. When AMP Stories were in the AMP plugin it was an issue because we stored stories in post content and rendered stories via the_content(). This, however, is no longer done in the Web Stories, so there is much less opportunity (or none) for plugins to corrupt the_content to cause an amp-story to fail in such a dramatic way. cc @swissspidy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Groomed P2 Low priority Sanitizers wontfix WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@westonruter @swissspidy @kmyram @ThierryA @jauyong