Make WordPress Core

Opened 15 years ago

Closed 9 years ago

Last modified 9 years ago

#12133 closed enhancement (fixed)

get_field_id() and get_field_name() break when passed a name in array format

Reported by: johnbillion's profile johnbillion Owned by: johnbillion's profile johnbillion
Milestone: 4.4 Priority: normal
Severity: minor Version: 2.9.1
Component: Widgets Keywords: has-patch has-unit-tests
Focuses: administration Cc:

Description

get_field_name() and get_field_id() are member functions of the widget API which generate the names and ids for fields on the widget admin screen.

A custom widget cannot use an array format name with this function.

For example, 'myfield[]' or 'myfield[3]' is a valid name for an input field, but this name cannot be passed to get_field_name() as the function will strip right square brackets from the name.

Patch upcoming when I find the time.

Attachments (4)

patch.diff (1.1 KB) - added by ch1902 12 years ago.
12133.diff (1.1 KB) - added by welcher 9 years ago.
Refreshing patch
12133.2.diff (1.1 KB) - added by welcher 9 years ago.
12133-tests.diff (2.8 KB) - added by welcher 9 years ago.

Download all attachments as: .zip

Change History (18)

#1 @scribu
15 years ago

  • Cc scribu@… added

#2 @scribu
15 years ago

  • Milestone changed from Unassigned to 3.0

#3 @nacin
15 years ago

  • Keywords needs-patch added; get_field_name get_field_id widgets removed
  • Milestone changed from 3.0 to Future Release

Needs patch, sending to future release for now.

@ch1902
12 years ago

#4 @ch1902
12 years ago

Not sure if I've done this right, it's my first patch.

This fixes the array style input names and makes get_field_id() produce tidier HTML id attributes. I realise HTML5 id attributes are much more flexible in what they accept than HTML4, but I think simpler IDs are better for jQuery/CSS selectors.

#5 @scribu
12 years ago

1) In get_field_id():

str_replace(array('][', '[', ']', ' '), '-', trim($field_name, ' []')

can be simplified to this:

strtr('[] ', '-', $field_name)

or maybe even

sanitize_title_with_dashes($field_name)

2) I don't think the logic in get_field_name() is correct. If $field_name is 'foo[bar]', you'll end up with nested brackets: [$this->id][foo[bar]] which will not produce correct values in $_POST.

3) Tabs, not spaces http://make.wordpress.org/core/handbook/coding-standards/#indentation

#6 @ch1902
12 years ago

1) No, but it could be simplified to

strtr($field_name, '[] ', '---')

However that produces less than attractive IDs such as "a-b--c-" from "a[b][c]" instead of my version which produces "a-b-c". I have no opinion on sanitize_title_with_dashes because I don't know much of the internals of Wordpress and was sticking to pure PHP.

2) The logic is correct and does not produce nested brackets, I did test on my local copy before submitting.

3) I didn't realise the source used tabs, my IDE auto converted them. I didn't see any problems with spaces in the other patches I could find but I'll remember if I ever dare to submit a patch again.

#7 @DrLightman
10 years ago

Is this still broken? If I use something like:

$this->get_field_name( 'posttypes[]' )

It doesn't work on WP 4.0.1

Last edited 10 years ago by DrLightman (previous) (diff)

#8 @wonderboymusic
9 years ago

  • Milestone changed from Future Release to 4.4
  • Owner changed from azaozz to johnbillion
  • Status changed from new to assigned

We should unit test this and see what we want to correct

@welcher
9 years ago

Refreshing patch

#9 @welcher
9 years ago

Adding updated unit tests with dataproviders containing original tests and the array like formats listed in the description. Original patch did not return the expected results for me.

@welcher
9 years ago

#10 @welcher
9 years ago

Adding new patch that passes the unit tests previously attached.

@welcher
9 years ago

#11 @welcher
9 years ago

  • Keywords has-patch dev-feedback needs-testing added; needs-patch removed

#12 @johnbillion
9 years ago

  • Focuses administration added
  • Keywords has-unit-tests added; dev-feedback needs-testing removed

Tested. Updating a widget's options that uses a field name with array notation now works as expected.

Note that it doesn't make as much sense for the get_field_id() method to support an array format name, but it works fine.

#13 @johnbillion
9 years ago

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

In 34780:

Introduce support for array format field names in WP_Widget::get_field_name() and WP_Widget::get_field_id().

Fixes #12133
Props ch1902, welcher

#14 @DrewAPicture
9 years ago

In 34782:

Docs: Fix the placement and ordering of some @since tags following [34780].

See #12133. See #32246.

Note: See TracTickets for help on using tickets.