WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 4 months ago

Last modified 4 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:

Description

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(
    menu_icon' => 'data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4NCjwhLS0gR2VuZXJhdG9yOiBBZG9iZSBJbGx1c3RyYXRvciAxNS4wLjAsIFNWRyBFeHBvcnQgUGx1Zy1JbiAuIFNWRyBWZXJzaW9uOiA2LjAwIEJ1aWxkIDApICAtLT4NCjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBTVkcgMS4xLy9FTiIgImh0dHA6Ly93d3cudzMub3JnL0dyYXBoaWNzL1NWRy8xLjEvRFREL3N2ZzExLmR0ZCI+DQo8c3ZnIHZlcnNpb249IjEuMSIgaWQ9IkxheWVyXzEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHg9IjBweCIgeT0iMHB4Ig0KCSB3aWR0aD0iMzU1LjAyNHB4IiBoZWlnaHQ9IjQzNS4wNnB4IiB2aWV3Qm94PSIwIDAgMzU1LjAyNCA0MzUuMDYiIGVuYWJsZS1iYWNrZ3JvdW5kPSJuZXcgMCAwIDM1NS4wMjQgNDM1LjA2Ig0KCSB4bWw6c3BhY2U9InByZXNlcnZlIj4NCjxnPg0KCTxnPg0KCQk8cGF0aCBmaWxsPSIjRkZGRkZGIiBkPSJNMTc5LjIzNywwQzEzOC45ODIsMCw4My4zODgsMTEuNTAyLDYwLjM4NCwyMS4wODdzLTM4LjM0LDE5LjE3LTQzLjEzMiw0My4xMzJMMCwxOTcuMDZ2MTgzLjA0NWgyOS43MTINCgkJCXYyOC43MDVjMCwzNSw1MS4yMTgsMzUsNTEuMjE4LDB2LTI4LjcwNWg5NC41NTdoMC4zaDk4LjMwN3YyOC43MDVjMCwzNSw1MS4yMTgsMzUsNTEuMjE4LDB2LTI4LjcwNWgyOS43MTJWMTk3LjA2TDMzNy43NzIsNjQuMjE5DQoJCQljLTQuNzkyLTIzLjk2Mi0yMC4xMjgtMzMuNTQ3LTQzLjEzMi00My4xMzJDMjcxLjYzNiwxMS41MDIsMjE2LjA0MiwwLDE3NS43ODcsMCIvPg0KCTwvZz4NCgk8Zz4NCgkJPGc+DQoJCQk8Zz4NCgkJCQk8Y2lyY2xlIGN4PSIyOTkuNjgyIiBjeT0iMjk4LjE1MyIgcj0iMjQuNTciLz4NCgkJCQk8cGF0aCBkPSJNMjk5LjY4MiwyOTguMTUzIi8+DQoJCQk8L2c+DQoJCQk8Zz4NCgkJCQk8Y2lyY2xlIGN4PSI1NS4zNDIiIGN5PSIyOTguMTUzIiByPSIyNC41NyIvPg0KCQkJCTxwYXRoIGQ9Ik01NS4zNDIsMjk4LjE1MyIvPg0KCQkJPC9nPg0KCQk8L2c+DQoJCTxnPg0KCQkJPHBhdGggZD0iTTE3NS42MTIsNTUuMTEzaC03MS4xMzZjLTE0LjM3OCwwLTE0LjM3OC0yMS41NjYsMC0yMS41NjZoNzEuMzExaDc0Ljc2MWMxNC4zNzgsMCwxNC4zNzgsMjEuNTY2LDAsMjEuNTY2SDE3NS42MTJ6Ii8+DQoJCTwvZz4NCgkJPGc+DQoJCQk8cGF0aCBkPSJNMTc1LjYxMiw3Ni44ODdINjUuNzE1Yy0xNS4xODgsMC0xOS4xNTYsNy43MTctMjEsMTkuNDIzbC0xMy40MjgsOTYuMzQ4Yy0xLjI1Miw5LjIzNSwxLjQxOSwxOC40MDIsMTQuMTc4LDE4LjQwMg0KCQkJCWgxMzAuMzIyaDEzMy43NzJjMTIuNzU5LDAsMTUuNDMtOS4xNjcsMTQuMTc4LTE4LjQwMkwzMTAuMzA5LDk2LjMxYy0xLjg0NC0xMS43MDYtNS44MTItMTkuNDIzLTIxLTE5LjQyM0gxNzUuNjEyeiIvPg0KCQk8L2c+DQoJPC9nPg0KPC9nPg0KPC9zdmc+DQo=',
);
register_post_type( 'buses', $args );

Attachments (8)

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

Download all attachments as: .zip

Change History (22)

comment:1 wpsmith8 months ago

  • Type changed from defect (bug) to enhancement

comment:2 SergeyBiryukov8 months ago

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

Duplicate of #19886, see also #19354.

comment:3 SergeyBiryukov8 months 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 nacin8 months 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 SergeyBiryukov8 months ago

  • Milestone changed from 3.7 to 3.8

comment:6 matt5 months ago

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

comment:7 nacin4 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 iammattthomas4 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.

helen4 months ago

helen4 months ago

comment:9 helen4 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

helen4 months ago

helen4 months ago

nacin4 months ago

comment:10 nacin4 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 nacin4 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.

helen4 months ago

helen4 months ago

comment:12 helen4 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: Rarst4 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 SergeyBiryukov4 months ago

Replying to Rarst:

Late night note from two horribly confused developers

Follow-up: #26617

Note: See TracTickets for help on using tickets.