WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 2 years 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 5 years ago.
17975.diff (3.7 KB) - added by kawauso 5 years ago.
type="text/css" for validation
17975.patch (2.1 KB) - added by dd32 4 years ago.
17975.button.patch (829 bytes) - added by SergeyBiryukov 4 years ago.
17975-create-config-button-style.diff (1.1 KB) - added by westi 4 years ago.
Inline style additions to fix the create a config button

Download all attachments as: .zip

Change History (25)

#1 @westi
5 years ago

  • Keywords has-patch commit added

@kawauso
5 years ago

type="text/css" for validation

#2 @kawauso
5 years ago

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

#3 @westi
4 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.

#4 @SergeyBiryukov
4 years ago

  • Milestone changed from Future Release to 3.3

#5 @dd32
4 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.

#6 @dd32
4 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..

@dd32
4 years ago

#7 @dd32
4 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.

#8 @westi
4 years ago

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

#9 @dd32
4 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..

#10 follow-up: @dd32
4 years ago

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

#11 in reply to: ↑ 10 @SergeyBiryukov
4 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.

#12 @ocean90
4 years ago

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

Related: #18866

#13 @ocean90
4 years ago

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

Closed as a dup #18866.

#14 @ocean90
4 years ago

Question is stylesheet or inline style?

#15 @SergeyBiryukov
4 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.

@westi
4 years ago

Inline style additions to fix the create a config button

#16 @westi
4 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.

#17 @SergeyBiryukov
4 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.

#18 @westi
4 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.

#19 @dd32
4 years ago

In [20209]:

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

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


2 years ago

Note: See TracTickets for help on using tickets.