Make get_search_form() more filterable/extensible

[chipbennett]

I notice that a lot of the time when Themes include custom search form markup via searchform.php, they are making very minor changes to the default markup. (A common on is adding onfocus/onblur attributes to the input.) Unfortunately, I've also noticed that custom search form markup tends to be... forgotten, and ends up becoming more insecure over time.

So, in the interest of encouraging/facilitating use of the core markup, I propose making get_search_form() more extensible, similar to the extensibility of comment_form().

This patch replaces $echo = true with $args = array() as the parameter passed to get_search_form(), while retaining backward compatibility with $echo = true. Also, a (self-explanatory) search_form_defaults filter is added.

Also addresses #16538

Does not address #16541, which is potentially related.

Probably conflicts with #14851

Also related: #19321

[chipbennett]

Replacement get_search_form(), with updated phpDocs

[toscho]

label and placeholder are different things. Imagine a screenreader user listening to: Search for: Search.

Enter one or more search terms would be slightly better; usually you just don’t need a placeholder.

There should be a space between the input elements.

Besides that, I think this is a very good enhancement.

[chipbennett]

Adds $templates array for locate_template(), to allow for searchform-$name.php. See #14851

[chipbennett]

Replying to toscho:

label and placeholder are different things. Imagine a screenreader user listening to: Search for: Search.

Enter one or more search terms would be slightly better; usually you just don’t need a placeholder.

I'm open to direction on this. Basically, I just incorporated #16538. What's the best approach?

There should be a space between the input elements.

Isn't there? I'm imploding the atts arrays with spaces before outputting them in the HTML markup.

[GaryJ]

Could you use submit_button()?

How about adding an ​aria landmark role as a default? (And if that's added, then adding an ID to the label (input field ID + '-label') and a aria-labelledby attribute to the input field would also increase the accessibility potential.)

Is there not a existing WP function that can take an array of HTML attributes name => value and build the string for you?

[GaryJ]

Replying to chipbennett:

Replying to toscho:

There should be a space between the input elements.

Isn't there? I'm imploding the atts arrays with spaces before outputting them in the HTML markup.

He's talking about the elements, not the attributes. I think I disagree though - visual whitespace can be added with CSS, and I'm not aware of any accessibility issues with them following right on from each other within the markup. Easier for developers to add some margin between the elements, than having to filter the output just to trim extra spaces that is being suggested to be added.

[toscho]

Replying to chipbennett:

I'm open to direction on this. Basically, I just incorporated #16538. What's the best approach?

For a simple search field, the best approach is to leave the placeholder out. The UI has to work without placeholderanyway, because not all browsers support it. You cannot ​style the `placeholder` in all browsers – and risk bad contrasts.
placeholder may be useful for difficult forms (taxes etc.), but a clearly labeled search form doesn’t need it. The filter for additional attributes is good enough.

There should be a space between the input elements.

Isn't there? I'm imploding the atts arrays with spaces before outputting them in the HTML markup.

        $form .= '<label for="' . esc_attr( $args['id'] ) . '" ' . $label_atts . '>' . esc_attr( $args['label_text'] ) . '</label>'; 
        $form .= '<input name="' . esc_attr( $args['id'] ) . '" id="' . esc_attr( $args['id'] ) . '" ' . $input_atts . ' />'; 
        $form .= '<input id="' . esc_attr( $args['form_id'] ) . '" ' . $submit_atts . ' />'; 

Search for:SearchSearch :)

[toscho]

In my themes I use accept-charset='<?php bloginfo( 'charset' ); ?>' for the form just to raise the possibility to get UTF-8.

And aria-required='true' required for the search field may prevent accidental clicks on the submit button.

[chipbennett]

How about adding an aria landmark role as a default? (And if that's added, then adding an ID to the label (input field ID + '-label') and a aria-labelledby attribute to the input field would also increase the accessibility potential.)

In my themes I use accept-charset='<?php bloginfo( 'charset' ); ?>' for the form just to raise the possibility to get UTF-8.

And aria-required='true' required for the search field may prevent accidental clicks on the submit button.

Since the $atts arrays are extensible/filterable, these can easily be added by the Theme developer. I've basically taken the form markup in the current version of get_search_form(), and replicated it here. The only difference is adding placeholder="search", which I now think should be removed, and left to Themes/Plugins to filter in.

He's talking about the elements, not the attributes. I think I disagree though - visual whitespace can be added with CSS, and I'm not aware of any accessibility issues with them following right on from each other within the markup. Easier for developers to add some margin between the elements, than having to filter the output just to trim extra spaces that is being suggested to be added.

Search for:SearchSearch :)

Unless I'm missing something, I agree with GaryJ that this is a presentational/CSS issue, that should be left up to Themes to determine how to implement.

Is there not a existing WP function that can take an array of HTML attributes name => value and build the string for you?

Is there? If you find one let me know. That would be ideal to use!

Could you use submit_button()?

I suppose I could - though I'd like some more core-dev feedback on that before adding it to the patch. I've only ever seen submit_button() used in the admin, not on the front end.

[chipbennett]

Remove placeholder="search" from default atts array. Reverts changes related to #16538

[ramiy]

Related: #15081

[WraithKenny]

Patch/plan should probably be updated with regards to the above related.