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.

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

Address an issue with an invalid embed #1661

Merged
merged 4 commits into from
Nov 27, 2018
Merged

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Nov 27, 2018

Steps To Reproduce

  1. Activate a theme that calls the_content inside a function, like the Genesis theme Essence Pro
  2. Create a new post using the block editor
  3. Add a 'Ted' block with this URL:
    https://www.ted.com/talks/derek_sivers_how_to_start_a_movement
  4. Save the post
  5. Visit the validation error page for that post
  6. Expected: 'Embed' appears as the sources, as the Ted block is the source of the validation errors:

expected-amp-embed

7. Actual: The theme is reported as the source

actual-parent-theme

Thanks to @hellofromtonya for reporting this.

The Genesis theme has a function genesis_do_post_content() that calls the_content():
do_post_content

...so it was reported as the source.

Still, I'm not sure this PR's approach is right.

…theme

This might not be the right approach,
but this addresses an issue that Tonya found
in testing this with the Essence Pro (Genesis) theme.
Genesis has a function genesis_do_post_content()
that calls the_content().
So if an embed in the post content has a validation error,
the theme will be reported as a source of a validation error.
So this outputs the theme as a source only if
there's no embed source of the validation error.
@westonruter
Copy link
Member

Good catch.

@westonruter westonruter added this to the v1.0 milestone Nov 27, 2018
@westonruter westonruter merged commit adf976a into 1.0 Nov 27, 2018
@westonruter westonruter deleted the update/embed-validation-error branch November 27, 2018 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants