WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 5 months ago

#48881 new defect (bug)

Fix inline SVG error with wp_admin_css_color

Reported by: KZeni Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch needs-testing reporter-feedback
Focuses: css, administration Cc:

Description

To get to the point, url-friendly-colour in wp-admin/css/colors/_admin.scss currently has:

@return '%23' + str-slice( '#{ $color }', 2, -1 );

but both Firefox and Safari (possibly others) are having an issue with CSS generated based on this (at least inline SVG assets).

Safari (and possibly others) doesn't like the fact that ' is used to wrap the str-slice color value since inline SVG assets are using ' to wrap the parameters/attributes so it then breaks the SVG's syntax by having that value use ' within itself being wrapped in a '.

Then, Firefox (and possibly others) doesn't like the fact that # is used within an inline SVG since it's expecting %23 as with the rest of the SVG being encoded.

I've found that switching this out for:

@return '%23' + str-slice( %23{ $color }, 2, -1 );

resolves these issues experienced by both of these web browsers (and possibly others.) I haven't tested thoroughly for any potential side-effects, but I don't foresee all that much given how little it seems like url-friendly-colour is used (with this then just resolving browser issues for its current usage.)

Btw, I came across this with the Admin Color Schemer plugin (with it patched for PHP 7.x per the approved code on its GitHub; not sure why it hasn't been pushed out to WP.org yet... the dev should maybe do that. Either way, this plugin is using WP's own _admin.scss and it's not creating compliant results, currently, and this should resolve that issue.)

Attachments (1)

Screen Shot 2020-06-22 at 2.43.23 PM.png (103.1 KB) - added by KZeni 5 months ago.
Screenshot of this issue in Safari (also doesn't show in Chrome & others.)

Download all attachments as: .zip

Change History (7)

#1 @SergeyBiryukov
12 months ago

  • Keywords reporter-feedback added

Hi there, welcome to WordPress Trac! Thanks for the ticket.

As far as I can see, url-friendly-colour is only used for color schemes in core, however the end result in the inline SVG after running npm run grunt build:css looks like %23e14d43 (as seen, for example, in the Midnight scheme), without an ' or unencoded #.

It might be that I'm misunderstanding the issue, could you describe the steps to reproduce it? Are you running the required build tasks for the CSS?

#2 @KZeni
11 months ago

https://github.com/helen/admin-color-schemer creates a copy of the wp-admin/css/colors/_admin.scss file to generate its custom color schemes (it then uses its copy of this file at wp-content/uploads/admin-color-schemer/_admin.scss.) This is where I encountered this issue (and confirmed just now that it is still an issue.) It shows the checked checkboxes as broken images instead of displaying the SVG as expected.

Again, just swapping out @return '%23' + str-slice( '#{ $color }', 2, -1 ); to instead be @return '%23' + str-slice( %23{ $color }, 2, -1 ); fixes the issue and doesn't appear to have any side-effects.

#3 @KZeni
9 months ago

I suppose the latest changes to this aspect of WordPress has my quick fix not working/applicable anymore.

Version 2, edited 9 months ago by KZeni (previous) (next) (diff)

@KZeni
5 months ago

Screenshot of this issue in Safari (also doesn't show in Chrome & others.)

#4 @KZeni
5 months ago

Added screenshot (see above). Also giving it an update to say this issue is still present. Also, the proposed patch here still resolves the issue with seemingly no side-effects (swap out @return '%23' + str-slice( '#{ $color }', 2, -1 ); to instead be @return '%23' + str-slice( %23{ $color }, 2, -1 ); in wp-admin/css/colors/_admin.scss.)

#5 @KZeni
5 months ago

It is possible that this only comes up when using phpsass with the file like https://github.com/helen/admin-color-schemer is (though there could potentially be others like it.) That said, I've yet to have a case where accommodating this potential scenario then breaks anything.

This ticket was mentioned in PR #348 on WordPress/wordpress-develop by KZeni.


5 months ago

Trac ticket: https://core.trac.wordpress.org/ticket/48881

The Trac ticket has a bunch of additional details regarding the potential issue being resolved here, screenshots, etc.

Note: See TracTickets for help on using tickets.