Add 'id' argument to wp_dropdown_pages to match 'name' (needed for use in widget settings)

[jeremyclarke]

Problem

wp_dropdown_pages outputs a <select> with all the pages on your site in <option>s that have the page id as their value. There is already an option in $args for 'name' but there is none for 'id'. Currently the 'name' argument is used for both the name= and id= attributes in the <select>.

IMHO this is an obvious oversight and should be corrected on principle. It also specifically presents a problem because as it stands wp_dropdown_pages can't be used in the settings for a widget in the new OO widgets API. To save a setting you need to give it both a name and id with the $this->get_field_id() and $this->get_field_name() methods, so wp_dropdown_pages should accept both parameters to be compatible.

Solution

This patch adds an 'id' argument to the defaults (with the same default as 'name', so it will be backwards compatible) and uses it in the output.

Interim Hack

For anyone else bumping up on this problem: Until this patch is in the core a temporary solution is to fetch the output using $args['name'] = $this->get_field_id('setting_name') and $args['echo'] = FALSE, then do a str_replace() on the output to change the name to be $this->get_field_name(). The same principle (fetch the output with echo turned off then filter the id/name to be what you need) can be applied to other situations where this patch would be useful.

Thanks for filosofo for help with the interim solution in #wordpress-dev.

[ShaneF]

Looks good here. Moved to 3.0 because of feature freeze. :)

[jeremyclarke]

12397 - update wp_dropdown_pages with id argument

[jeremyclarke]

I just updated the attachment to reflect #wordpress-dev feedback from filosofo.

The original patch set $id as 'page_id' in $defaults along with $name (which ensures back-compat with people who didn't set a $name argument). Filosofo pointed out that if someone had used the old $name argument and then styled/javascripted the resulting <select> based on the assumption that $name would also control the #id, my patch would break their code, because the id would now be 'page_id' rather their their $name parameter.

The new patch instead sets 'id' => '' in the $defaults definition, then later, IF there was no id parameter passed in, it sets $id = $name. This way the ID will work but only if its explicitly set.

IMHO this is not the ideal solution because it is a strange and idiosyncratic behavior, but in the name of backwards-compatibility it is probably safer to go with this. If there is a committer who would rather have an elegant wp_dropdown_pages() than a 100% back-compat then feel free to switch it back to the other way. Since $_POST is controlled by 'name' and not 'id' it seems that most uses of wp_dropdown_pages() would not be effected by either patch.

[hakre]

name and id are technically the same. it might be that this bug report is invalid.

[filosofo]

Replying to hakre:

name and id are technically the same.

I'm not sure why you would say this. Their definitions and standard data types differ (CDATA for name, NAME for id).

Also, their practical uses differ: if someone wants to have a pages select element appear in multiple places in the document, he must change the id attribute but not necessarily the name attribute.

[jeremyclarke]

Replying to hakre:

name and id are technically the same. it might be that this bug report is invalid.

In the new APIs being added to wordpress form elements are being given name values that set up arrays, like name="myoption[setting1]" and id's that mirror the names but with underscores, like id="myoption_setting1". When you start passing names that function as array definitions they don't really work as id's anymore, so they need to be different.

This is why wp_dropdown_pages doesn't work with the Widgets API right now, it just makes sense to let people give seperate names and ids. The current patch also keeps the default behavior, so if you don't specify an id the name value is used. If you like making forms with id matching name you still can and won't have to add a new argument.

[hakre]

Replying to filosofo:

Replying to hakre:

name and id are technically the same.

I'm not sure why you would say this. Their definitions and standard data types differ (CDATA for name, NAME for id).

What I meant is that it makes no difference wether I pass a name or id attribute to a form element and therefore the report might have been invalid, I just needed clarification. Both of these attributes are designed to be used as fragment identifiers. Name attribute is depreceated in xhtml 1.0 on certain elements (incl. a and form but not input elements AFAIK) and gets more love again in HTML 5 ​Ref, for the HTML 4.01 standarization you're right for their datatype definition (name = cdata [CI] ​Ref; id = name [CS] ​Ref) but if you switch over to XHTML 1.0 you will notice that name has been changed to a new definition (name = NMTOKEN).

So I wrote that because it is kind of common good to put the same value into ID and NAME most of all the time (or drop the one or other). I'm aware that this can collide while using the ​html form names and php array feature while checking for a document's ​validity. To get more information on compability issues regarding ID and NAME the XHTML 1.0 specs have an ​informative appendix which covers most of the connected areas. I'm as well aware that it's not a must to do so.

Also, their practical uses differ: if someone wants to have a pages select element appear in multiple places in the document, he must change the id attribute but not necessarily the name attribute.

This can but must not be done. The backend can deal with different names for different form elements doing the same as well, for XHTML it is very much advised to have an ID compareable value in name as well (ID). Again, I do not say, that this must be in WP, I just wanted to get more info regarding the report.

jeremyclarke differentiated already with his comment (after yours), so I now better understand what this is about.

---

Replying to jeremyclarke:

Replying to hakre:

name and id are technically the same. it might be that this bug report is invalid.

In the new APIs being added to wordpress form elements are being given name values that set up arrays, like name="myoption[setting1]" and id's that mirror the names but with underscores, like id="myoption_setting1". When you start passing names that function as array definitions they don't really work as id's anymore, so they need to be different.

Makes sense, I was and I am not aware of a new WordPress Form API so therefore I did not know and was not able to test against it. After your calrification I tested it right away and it works.

If you can provide references to the new API for form elements you were talking about I would be able to test the patch against those as well.

This is why wp_dropdown_pages doesn't work with the Widgets API right now, it just makes sense to let people give seperate names and ids. The current patch also keeps the default behavior, so if you don't specify an id the name value is used. If you like making forms with id matching name you still can and won't have to add a new argument.

Sounds very reasonable to me, but I tested your patch and I must say: It does not make any difference for me. I tested on the page-editor with the page-dropdown in the attributes widget there. It was one of the two points where wp_dropdown_pages is actually used (the other one is the inline editor):

without patch:

<select id="parent_id" name="parent_id">
	<option value="">Main Page (no parent)</option>
	<option value="2" class="level-0">About</option>
	<option value="45" class="level-1">   My Cat</option>
	<option value="47" class="level-1">   My Car</option>
</select>

with patch:

<select id="parent_id" name="parent_id">
	<option value="">Main Page (no parent)</option>
	<option value="2" class="level-0">About</option>
	<option value="45" class="level-1">   My Cat</option>
	<option value="47" class="level-1">   My Car</option>
</select>

That might not be the test-case you thought about when I read your report. Please provide a test-case so your newly added feature can be properly tested.

As far as I see this patch is working anyway, so I add the tested tag.

[jeremyclarke]

Replying to hakre:

Thanks for your careful consideration. I hear what you're saying about how they should be the same, and it would have been a good argument against the changes that have been made to the widgets API to use the new OO model for creating new widgets. That is the 'form api' I'm talking about, the system where you extend WP_Widget to add new widgets. It uses two methods, WP_Widget::form and WP_Widget::update, to automate the process of setting options for each widget instance. This involves using two other methods to get the id and name of each field in the form, $this->get_field_id('badges_page_id') and $this->get_name_id('badges_page_id').

That's the test case I'm talking about. They output different things as mentioned above, name is the standard output like name="myoption[setting1]" and id's mirror the names but with underscores, like id="myoption_setting1". Thus its not a new form api per se, but this effectively creates a situation where WP needs to make any form-related functions work with seperate names and ids, so that they will be compatible with widget settings, which are some of the most important in WP going forward (widget api is basically the CMS api :P)

In this situation it's impossible to use wp_dropdown_pages because the name and ID must be different and the function doesn't allow it. I want to use wp_dropdown_pages in my widget settings because the widget needs to know the ID of a specific page.

In case its not clear, I'm using wp_dropdown_pages in my plugin code as part of the API, rather than patching it for its meager uses in core. IMHO this function is incredibly useful for plugin authors and should be treated as public API, to force plugins to redo the work of this function makes no sense to me. It's a great, easy to use function in all other situations and it should work with widgets too. The simplest possible test case would probably be a 'show page contents' widget, where you choose a page who's contents you want to show inside the widget.

[ryan]

(In [12882]) Add id arg to wp_dropdown_pages(). Props jeremyclarke. fixes #11433