Skip to content

Carry metabox icons as their own field instead of concatenating markup into the title #38

Description

@donnchawp

Follow-up to #37.

Metaboxes have had a $metaboxIcon property since 2.98.7, but initMetabox() throws it away immediately and concatenates the markup into the title string instead:

// lazy hackaround for now, can be more classy later.
if ( ! empty( $this->metaboxIcon ) ) {
	$this->metaboxTitle = '<i class="' . $this->metaboxIcon . ' icon"></i> ' . $this->metaboxTitle;
}

That's includes/ZeroBSCRM.MetaBox.php:71-74. The comment is honest about it.

The result is that $box['title'] is an HTML string, and every place that renders it has to choose between escaping the title (which shows a literal <i class="heartbeat icon"></i> to the user) or not escaping it (which makes the title an HTML sink forever). #37 fixes one of those places with a narrow wp_kses() allowlist. The others are still inconsistent:

  • ZeroBSCRM.MetaBox.php:881 - the drag-drop blocker overlay still uses esc_html(), so the raw tag is visible while rearranging metaboxes.
  • ZeroBSCRM.MetaBox.php:710 - the tab head does too. Not reachable today, since both Activity metaboxes inherit 'can_become_tab' => false, but it's waiting for the next icon-bearing box.
  • ZeroBSCRM.ScreenOptions.php:101, :105, :135, :138 - these echo the title with no escaping at all, which is why the icon already renders there.

The fix is to stop merging the two values and carry the icon as its own key.

  • Drop the concatenation at :71-74 and pass $this->metaboxIcon down through create_meta_box() (both call sites, :107 and :125).
  • Add an $icon = '' parameter to zeroBSCRM_add_meta_box() at :298. It already takes ten positional arguments, so appending keeps any third-party callers working.
  • Forward it in the recursive array-of-screens call at :315, or metaboxes registered against multiple screens quietly lose their icon.
  • Store 'icon' => $icon in the box array at :402, and restore it in the 'sorted' priority branch at :365-372 alongside the other six keys. That branch is dead code at the moment (its only caller, :451, is commented out) but it should stay consistent.
  • Have each render point emit the icon itself and keep esc_html() on the title. Something like:
function jpcrm_metabox_icon_html( $box ) {
	if ( empty( $box['icon'] ) ) {
		return '';
	}
	return '<i class="' . esc_attr( $box['icon'] ) . ' icon" aria-hidden="true"></i> ';
}
  • While in ZeroBSCRM.ScreenOptions.php, add the missing esc_html() / esc_attr() on $mbTitle and $mbID. That's a real escaping gap, not tidying.

Two files, and the subclasses don't change at all. Only ZeroBSCRM.MetaBoxes3.Contacts.php:2441 and ZeroBSCRM.MetaBoxes3.Companies.php:1043 set an icon, and both just set 'heartbeat'.

The thing I'm not sure about is extensions. If any extension subclasses zeroBS__Metabox and hardcodes <i> markup into metaboxTitle directly rather than using metaboxIcon, it renders fine after #37 and would start showing literal tags after this change. I can't check that from this repo, so it's worth a look before starting.

There are no metabox tests in tests/php/, and the render points are bare echo calls inside a long procedural function, so testing this properly means extracting the header rendering first. That's part of the same job rather than a reason to skip it.

Feedback welcome if there's a cleaner shape for this.

Metadata

Metadata

Assignees

No one assigned

    Labels

    ready-for-agentFully specified, ready for an AFK agent

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions