Make WordPress Core

Opened 13 years ago

Closed 12 years ago

Last modified 10 years ago

#17975 closed defect (bug) (fixed)

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

Reported by: westi's profile westi Owned by: westi's profile 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 13 years ago.
17975.diff (3.7 KB) - added by kawauso 13 years ago.
type="text/css" for validation
17975.patch (2.1 KB) - added by dd32 13 years ago.
17975.button.patch (829 bytes) - added by SergeyBiryukov 12 years ago.
17975-create-config-button-style.diff (1.1 KB) - added by westi 12 years ago.
Inline style additions to fix the create a config button

Download all attachments as: .zip

Change History (25)

#1 @westi
13 years ago

  • Keywords has-patch commit added

@kawauso
13 years ago

type="text/css" for validation

#2 @kawauso
13 years ago

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

#3 @westi
13 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
13 years ago

  • Milestone changed from Future Release to 3.3

#5 @dd32
13 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
13 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
13 years ago

#7 @dd32
13 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
13 years ago

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

#9 @dd32
13 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
13 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
13 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
12 years ago

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

Related: #18866

#13 @ocean90
12 years ago

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

Closed as a dup #18866.

#14 @ocean90
12 years ago

Question is stylesheet or inline style?

#15 @SergeyBiryukov
12 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
12 years ago

Inline style additions to fix the create a config button

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


10 years ago

Note: See TracTickets for help on using tickets.