WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 2 months ago

#46910 accepted defect (bug)

Allow wp_die to called after html has started output

Reported by: spacedmonkey Owned by: SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.2
Component: Site Health Keywords: servehappy has-patch
Focuses: Cc:

Description

Currently if wp_die is called in either front end or in the cms, after html has started to render html, it makes an extremely broken looking page. This is because the default wp_die will output another html / body tag and css.

The wp_die function could somehow detect that html render had started, by saying checking an action has been run or a constraint was set, then instead of outputting the html start tags and would instead just output Javascript. This Javascript could display either an overlay dialog or even an a pure Javascript based alert displaying the message.

This feature would be specially useful for the fatal error protection feature in WordPress 5.2.

Attachments (1)

46910.diff (1.1 KB) - added by spacedmonkey 4 months ago.

Download all attachments as: .zip

Change History (14)

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


4 months ago

#2 @ocean90
4 months ago

The default wp_die handler already checks for did_action( 'admin_head' ) to not print the opening tags and CSS in admin. But it still always prints the closing tags which should probably only be done if exit is set.

#4 @spacedmonkey
4 months ago

@ocean90 Good spot. But doesn’t do the same for the thing front end. The check for did_action(‘admin_head’) could be changed for check for wp_head as well. Meaning, the wp_die would work much better on the front end. But it might mean styling would look weird, but this maybe fixable by theme developers...

#5 @SergeyBiryukov
4 months ago

  • Component changed from General to Bootstrap/Load
  • Milestone changed from Awaiting Review to 5.2
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


4 months ago

#7 @flixos90
4 months ago

#46811 fixed this for the admin, let's continue exploring here for how to cover the frontend.

@spacedmonkey
4 months ago

#8 @spacedmonkey
4 months ago

  • Keywords has-patch added; needs-patch removed

Uploaded the first patch 46910.diff as a talking point.

This is a very simple patch, that just makes sure that html / body tags are not re-opened on the front end. All it does it makes you an unstyle div with the error message is displayed on the page. Personally, I think this is what the end patch should look like. I don't like the idea of trying to build a dialog that works on every theme ever write. I think we should just output a div with an id, and leave it up the theme creates to style it (if they so wish), in a way that fits in with the styling of the theme.

#9 @pento
4 months ago

  • Milestone changed from 5.2 to 5.3

This patch needs further discussion, so bumping to 5.3.

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


3 months ago

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


2 months ago

#12 @dimadin
2 months ago

I have reviewed this patch and here is what I found:

  • First check doesn't work on frontend until second && (before ! did_action( 'wp_head' )) is not replaced with ||.
  • when you use wp_die() on wp_head, there is no any styling as you don't get CSS styles anymore.
  • wp_body_open check will fail on most themes.
  • Any reason why wrapper for error message is part of this?

#13 @spacedmonkey
2 months ago

  • Component changed from Bootstrap/Load to Site Health
Note: See TracTickets for help on using tickets.