WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 7 weeks ago

#17975 closed defect (bug) (fixed)

_default_wp_die_handler css referencing logic is fragile and doesn't always work

Reported by: westi Owned by: westi
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2
Component: General Keywords: needs-patch
Focuses: Cc:

Description

The logic for determining where the install and install-rtl css files are in the default wp_die handler is fragile and breaks relatively easily.

For example calling wp_die in sunrise.php in a multisite install when someone accesses anywhere except for the root of a site.

The best solution to this is to move the CSS inline and remove all the wp-admin determining logic from the code.

Attachments (5)

wp-die-inline-css.diff (3.7 KB) - added by westi 3 years ago.
17975.diff (3.7 KB) - added by kawauso 3 years ago.
type="text/css" for validation
17975.patch (2.1 KB) - added by dd32 3 years ago.
17975.button.patch (829 bytes) - added by SergeyBiryukov 2 years ago.
17975-create-config-button-style.diff (1.1 KB) - added by westi 2 years ago.
Inline style additions to fix the create a config button

Download all attachments as: .zip

Change History (25)

westi3 years ago

comment:1 westi3 years ago

  • Keywords has-patch commit added

kawauso3 years ago

type="text/css" for validation

comment:2 kawauso3 years ago

Added a patch with type="text/css" for the <style> tag because I'm a pedant.

comment:3 westi3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

In [18534]:

Move the CSS inline to wp_die so that it is always available. Fixes #17975, props kawauso for the style type update.

comment:4 SergeyBiryukov3 years ago

  • Milestone changed from Future Release to 3.3

comment:5 dd323 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This has removed some styles which are used by the config creater, The installer error pages might also be affected.

Specifically, h1 styling, list styling, button styling and the fonts don't seem right, Simplest way to see this is to attempt an install with incorrect credentials, the next page uses the most missing styles.

comment:6 dd323 years ago

Once I've started going through this, I've got at least another 100 lines of CSS which the installer uses.

My suggestion here is to re-instate the wp-admin guessing, or at least part of it, If we can tell for 100% certaintity the location of the CSS files, use them, otherwise fall back to the minimal inline styles.

POC patch coming up..

dd323 years ago

comment:7 dd323 years ago

Another reason why the installer can't just have all the CSS inline, the Go back button backgrounds are set via CSS backgrounds, causing the admin path needing to be known anyway.

An alternate suggestion would be to pass the admin stylesheet in via the $args, and fallback to the inline styles if thats not provided.. but feels like that'd break too under certain conditions.

comment:8 westi3 years ago

maybe the installer should not use wp_die to displayerror messages and instead display the itself?

comment:9 dd323 years ago

maybe the installer should not use wp_die to displayerror messages and instead display the itself?

I completely agree with you there actually, the validation errors should be inline, and i'll be attacking that in a seperate ticket shortly i think.

That being said, The wp_die() instance I was refering to there comes directly from wpdb, and I expect a number of plugins which utilise wp_die() expect the h1 and button stylings at least.

The suggested patch from me could be vastly simplified, instead, if it just checked for admin_url() being valid and enqueued the stylesheet then perhaps..

comment:10 follow-up: dd323 years ago

The wp_die styling changes have also affected The "no configuration file exists" screen along with the Database upgrade screen.

comment:11 in reply to: ↑ 10 SergeyBiryukov3 years ago

Replying to dd32:

The wp_die styling changes have also affected The "no configuration file exists" screen

Just stumbled upon this while submitting #18576. In particular, link colors and button styles are missing.

comment:12 ocean902 years ago

  • Keywords 2nd-opinion added; 3.3-early westi-likes commit removed

Related: #18866

comment:13 ocean902 years ago

  • Keywords needs-refresh dev-feedback added; has-patch 2nd-opinion removed

Closed as a dup #18866.

comment:14 ocean902 years ago

Question is stylesheet or inline style?

comment:15 SergeyBiryukov2 years ago

Styles for installation screens were revamped in [19297].

The only regression is that "Create a Configuration File" is now a link rather than a button, the rest seems good.

westi2 years ago

Inline style additions to fix the create a config button

comment:16 westi2 years ago

  • Keywords needs-patch added; needs-refresh dev-feedback removed

17975-create-config-button-style.diff fixed the create a config button for me.

If everyone is happy with this patch I think we can then close off this ticket.

comment:17 SergeyBiryukov2 years ago

17975.button.patch is a slightly updated olleicua's patch from #18866. Removed unneeded vendor prefixes for border-radius as per azaozz's comment in #18576.

17975-create-config-button-style.diff seems to be more comprehensive and matches 3.2 style better.

comment:18 westi2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [19417]:

Add button styles to the inline styles in _default_wp_die_handler so that the Create a Config button is styled again. Fixes #17975.

comment:19 dd322 years ago

In [20209]:

Restore the h1 styling for wp_die(), primarily affects database down messages. See #17975

comment:20 ircbot7 weeks ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

Note: See TracTickets for help on using tickets.