Make WordPress Core

Opened 15 years ago

Closed 10 years ago

Last modified 10 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 10 years ago.
Refreshing patch
12133.2.diff (1.1 KB) - added by welcher 10 years ago.
12133-tests.diff (2.8 KB) - added by welcher 10 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
10 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
10 years ago

Refreshing patch

#9 @welcher
10 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
10 years ago

#10 @welcher
10 years ago

Adding new patch that passes the unit tests previously attached.

@welcher
10 years ago

#11 @welcher
10 years ago

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

#12 @johnbillion
10 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
10 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
10 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.