Make WordPress Core

Opened 2 years ago

Closed 21 months ago

Last modified 21 months ago

#25147 closed enhancement (fixed)

Allow base64 data:image with Menu Icons

Reported by: wpsmith Owned by: nacin
Milestone: 3.8 Priority: high
Severity: minor Version:
Component: Administration Keywords: needs-patch
Focuses: Cc:


Currently, register_post_type allows the custom post type to set their menu icon via URL. It would be great to allow this parameter to accept a data string.

So, it could look something like this:

$args = array(
register_post_type( 'buses', $args );

Attachments (8)

25147.menu_icon_data_image_v1.patch (1.6 KB) - added by wpsmith 2 years ago.
25147.diff (6.0 KB) - added by helen 21 months ago.
25147.2.diff (6.3 KB) - added by helen 21 months ago.
25147.3.diff (6.4 KB) - added by helen 21 months ago.
25147.4.diff (8.8 KB) - added by helen 21 months ago.
25147.5.diff (8.8 KB) - added by nacin 21 months ago.
25147.6.diff (1.0 KB) - added by helen 21 months ago.
25147.7.diff (1.0 KB) - added by helen 21 months ago.

Download all attachments as: .zip

Change History (22)

comment:1 @wpsmith2 years ago

  • Type changed from defect (bug) to enhancement

comment:2 @SergeyBiryukov2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #19886, see also #19354.

comment:3 @SergeyBiryukov2 years ago

  • Milestone set to 3.7
  • Resolution duplicate deleted
  • Status changed from closed to reopened

On second thought, we could probably resolve this separately from #19354.

Reopening and moving for additional review.

comment:4 @nacin2 years ago

I'm thinking this should wait until 3.8 & MP6, which will likely significantly change how menu icons work. Might argue against a new enhancement here for now.

comment:5 @SergeyBiryukov2 years ago

  • Milestone changed from 3.7 to 3.8

comment:6 @matt22 months ago

  • Priority changed from normal to lowest
  • Severity changed from normal to minor

comment:7 @nacin21 months ago

  • Priority changed from lowest to high

This ticket highlights that our own PHP APIs have not caught up with brand new 3.8 best practices. That's not good. register_post_type() and add_menu_page() (et al.) need to be looked at.

comment:8 @iammattthomas21 months ago

I can't think of any reason not to do this -- though it could affect how SVG painter recolors menu icons, so someone who knows how that works should weigh in.

@helen21 months ago

@helen21 months ago

comment:9 @helen21 months ago

25147.2.diff allows a data:image URI resource or a Dashicon to be specified for the menu icon in both register_post_type() and add_menu_page(). Dashicons are specified using the helper class, e.g. dashicon-piechart.

Specifying the data:image URI directly in PHP isn't insecure, as if you're doing stuff in PHP you can do a lot of other things, but we may want to think about how this could affect plugins that have a GUI for custom post type management.

Some IRC discussion / thoughts, in particular:

nacin: wondering if we should have some kind of esc_menu_icon() function to handle all of this validation + a filter
nacin: wp_parse_menu_icon() perhaps

@helen21 months ago

@helen21 months ago

@nacin21 months ago

comment:10 @nacin21 months ago

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

In 26664:

Allow for Dashicons and base64-encoded data:image/svg+xml URIs when specifying menu icons.

Both of these icons can be colored to match the color scheme, including hover states.
Both are accepted for register_post_type()'s menu_icon argument, and also add_menu_page()'s $icon_url argument.

To use a Dashicon, pass the name of the helper class, e.g. 'dashicons-piechart'.
To use an SVG, pass a valid data URI string starting with 'data:image/svg+xml;base64,'.

props helen.
fixes #25147.

comment:11 @nacin21 months ago

  • Keywords needs-patch added; has-patch dev-feedback removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

The background-position: center will break plugins who are doing the CSS themselves with a single icon and a sprite. For example, https://wordpress.org/wp-content/plugins/jetpack/_inc/images/menuicon-sprite.png. This was previously best practice.

What exactly is the requirement here? I imagine we could only target dashicons if that's the thing that needs it.

@helen21 months ago

@helen21 months ago

comment:12 @helen21 months ago

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

In 26671:

More targeted background image styling for admin menu SVG icons. fixes #25147.

comment:13 follow-up: @Rarst21 months ago

Late night note from two horribly confused developers - inline docs went into release suggesting dashicons-piechart example, but the actual CSS is now dashicons-chart-pie. So anyone trying to follow example literally will be third horribly confused developer.

comment:14 in reply to: ↑ 13 @SergeyBiryukov21 months ago

Replying to Rarst:

Late night note from two horribly confused developers

Follow-up: #26617

Note: See TracTickets for help on using tickets.