Make WordPress Core

Opened 8 years ago

Closed 7 years ago

#34891 closed defect (bug) (fixed)

get_submit_button uses incorrect check for delete class

Reported by: rmccue's profile rmccue Owned by: afercia's profile afercia
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Administration Keywords: ui-feedback has-patch
Focuses: ui Cc:

Description

get_submit_button has a special case for the delete type:

if ( 'delete' === $type )
	$class = 'button-secondary delete';

However, type can never actually be 'delete', as it's always an array:

if ( ! is_array( $type ) )
	$type = explode( ' ', $type );

I'm not sure this is actually useful to fix though, since .delete doesn't seem to have any styling.

Attachments (1)

34891.patch (597 bytes) - added by sstoqnov 7 years ago.

Download all attachments as: .zip

Change History (7)

#1 @DrewAPicture
8 years ago

  • Keywords ui-feedback added

#2 @morganestes
8 years ago

Came across this today and wanted to chime in. I also didn't find any styles for .delete that would apply to this input, but since the 'delete' === $type is always false, the classes applied to the element are button delete instead of button-secondary delete, and that could end up affecting the UI.

However in the case I ran into today, both .button and .button-secondary get the exact same styles applied unless true is passed to get_submit_button() to wrap the input in a p, but the only difference there is a slight adjustment to vertical-align.

In searching for places that use 'delete' in core, I found only one use: in widgets.php ("Clear Inactive Widgets"). I'm assuming this hasn't cropped up sooner since that form relies on the removeinactivewidgets value to trigger its purpose and ignores the classes set.

#3 @afercia
8 years ago

  • Keywords needs-patch added

Seems the relevant changeset is [21789]: there was a switch before the change and as far as I see the delete case was there just to ensure the delete class was always paired with button-secondary. Maybe those two lines could be removed now since button-secondary should not be used (it's not responsive and doesn't reset the outline property on focus) and the default behaviour is to always have the base button class so the result is button delete.

@sstoqnov
7 years ago

#4 @sstoqnov
7 years ago

  • Keywords has-patch added; needs-patch removed

The two lines have been removed, since the type is always an array and this check is useless.

#5 @afercia
7 years ago

  • Focuses administration removed
  • Milestone changed from Awaiting Review to 4.7
  • Owner set to afercia
  • Status changed from new to reviewing

#6 @afercia
7 years ago

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

In 39059:

Administration: remove a leftover check from get_submit_button.

Props sstoqnov.
Fixes #34891.

Note: See TracTickets for help on using tickets.