Wp-calypso: Editor: Barnsbury page layout breaks editor in sandboxed site with Gutenberg 8.7

[p-jackson]

Steps to reproduce

1. Sandbox a test site (sandbox should be up-to-date enough that it has GB 8.7)
2. Create a new page
3. Choose the Barnsbury page layout (the vegetable one in the Home Pages category)
4. Publish the page (not necessary to reproduce, we just want to see what happens on the frontend too)
5. Refresh the page

What I expected

The editor reloads as expected. If you view the newly published page on front end you see the Barnsbury page layout. This is what happens when the site isn't sandboxed, so users aren't seeing the issue reported here.

What happened instead

After refreshing the editor placeholder appears for a bit but then the screen goes blank and fails to load.

If you load the page on the front end the page is filled with DOMDocument::loadHTML(): Tag time invalid in Entity warnings. I wonder if it's the editor trying to

Browser / OS version

Using GB 8.7. I believe this is the culprit because when I downgrade to 8.5.1 in the sandbox the warnings go away.

Screenshot / Video

Screenshot of the front end

[andrewserong]

@ramonjd and I encountered this issue using the Stratford page layout, with the latest posts block set to display post date. If we access the site from the front-end, we get the same error:

And in the backend, it results in a white page and the editor won't load receiveEntityRecords throws the following error:

I think this would be because the PHP warning is being outputted to the response from the server. Do we know why these warnings are being outputted in the sandbox instead of being suppressed or being sent to error logs instead of being part of the response?

This error that we could replicate is only happening for us while sandboxed.

[andrewserong]

Okay, my current hunch is that we're seeing this warning now due to the change in https://github.com/WordPress/gutenberg/pull/24351 where gutenberg_apply_block_supports is now applied to these server rendered blocks (and in turn then calls DOMDocument::loadHTML(). It looks like previously the gutenberg_experimental_apply_classnames_and_styles function might have bailed earlier before than after that PR landed (I can see that previously it'd return $block_content if there was an empty $block_type->supports, so I wonder if that's a clue).

At any rate, in https://github.com/WordPress/gutenberg/blob/master/lib/block-supports/index.php#L54 DOMDocument::loadHTML() is suppressed using the @ error control operator, but I'm wondering if it'd work better to use libxml_use_internal_errors for this kind of suppression so that debug-like environments don't get this warning in the output? (That said, I'm not too sure how either approach here works internally)

@Addison-Stavlo I see you reviewed https://github.com/WordPress/gutenberg/pull/24351, so I wondered if you might have more context here? CC: @ramonjd

[ramonjd]

is suppressed using the @ error control operator, but I'm wondering if it'd work better to use libxml_use_internal_errors

Thanks for getting to the bottom of it @andrewserong

I don't agree with using @ in general. It's convenient to suppress annoying errors, but it will also suppress errors that we probably don't expect and _want_ to know about.

But DOMDocument does not support HTML5 so we're going to see more of this in development environments if we don't turn it off fully.

Therefore +1 on using libxml_use_internal_errors either to disable or enable user error handling.

[ramonjd]

Do we know why these warnings are being outputted in the sandbox instead of being suppressed or being sent to error logs instead of being part of the response?

We're setting our own error handler via set_error_handler in the config See https://stackoverflow.com/a/6621834

🤔 ~Wonder if that's it?~

Confirmed. Commenting out our error handler allows @ to suppress the errors.

See 26096-pb for where we call set_error_handler

[lsl]

@andrewserong this wasn't fixed in WordPress/gutenberg/pull/24645, creating a new PR for this as I think it should probably be changed.

[andrewserong]

Thanks so much @lsl!

[lsl]

WordPress/gutenberg/pull/24677

[lsl]

This should be fixed in 8.9 - https://github.com/WordPress/gutenberg/commit/33c83fa7bd917704cf23cfc7106598c11fd0c6e5