Add a dedicated #autocomplete property to Form API form elements

subscribe.

Looks good to me. We should doc in the FAPI reference that the only useful value here is 'off'. That's what the tests do so we have minimal doc already.

Go Everett go! :)

There's one minor issue with this:

+++ b/modules/simpletest/tests/form_test.moduleundefined
@@ -987,6 +993,29 @@ function form_test_select_submit($form, &$form_state) {
+
+    $form['textfield'] = array(
+    '#type' => 'textfield',
+    '#title' => 'textfield',
+    '#autocomplete' => 'off',
+  );

The indentation needs to be fixed here. The $form line is indented 4 spaces instead of 2, and the properties need some indenting too. Otherwise, looks good.

Thanks Everett. Back to RTBC.

+ *     #placeholder, #autocomplete, #required, #attributes, #autocomplete_path.

This is going to be very confusing having #autocomplete/#autocomplete_path mean two completely different things that are not connected to each other.

http://api.drupal.org/api/drupal/includes--form.inc/function/theme_textf...

If we cleaned that up though, we might be able to remove #autcomplete_path from this list and reserve it for actual HTML stuff.

This doesn't necessarily need to depend on that clean up, but I'd like to find and/or open the issue to clean that up and have a decent plan of action in there (like a real autocomplete element or something). #125231: Enhance autocomplete feature looks like it might be the issue, although I feel like there's one for removing it from theme_textfield() somewhere too maybe.

Moving to needs work since we could at least use a comment or @todo explaining that the two have nothing to do with each other.

+++ b/includes/form.incundefined
@@ -2743,6 +2743,7 @@ function form_process_password_confirm($element) {
+    '#autocomplete' => empty($element['#autocomplete']) ? NULL : $element['#autocomplete'],

+++ b/includes/form.incundefined
@@ -2750,6 +2751,7 @@ function form_process_password_confirm($element) {
+    '#autocomplete' => empty($element['#autocomplete']) ? NULL : $element['#autocomplete'],

The empty check shouldn't be needed.

Also if we're adding new form property to the element we should declare it in element declaration in system_element_info. This will eliminate the need to the empty check.

Rerolled for core/.

I'm really not sure whether we need this. The autocomplete="off" attribute is used very rarely only.

The only difference this patch makes is that you specify '#autocomplete' => 'off' instead of '#attributes' => array('autocomplete' => 'off').

And for that, it needs to introduce explicit code in the theme functions of respective form elements to copy over the #property into #attributes, which means that #autocomplete only works for elements that support it explicitly - whereas #attributes][autocomplete works for any element.

It doesn't even process a more natural Boolean TRUE value into "no attribute" and FALSE into the special 'off' value. If that additional processing was contained, then I could vaguely see a DX point in adding #autocomplete.

Overall, this and some other related issues really cry for a Form API design specification that clarifies where we want to go and how much more code we want to introduce for dedicated #properties that map 1:1 to a key in #attributes, and/or what amount of (pre-)processing mandates a dedicated #property. Every dedicated #property adds to the code bloat issue in Form API, and also overall Drupal, since we suddenly also need dedicated tests for an element attribute that was formerly just simply part of #attributes.

Apparently, since the current incarnation of this patch only takes over the #autocomplete value as is into attributes, the tests being added here are merely testing the expected functionality of element_set_attributes(), which is sufficiently covered by other tests already. In other words: With the currently proposed #autocomplete "functionality", I don't see the point in adding tests -- they're needless, as they don't test any functionality that wouldn't be tested elsewhere already.

So then should we close (won't fix) this issue. Is there a purpose for using HTML5's auto-complete over our current implementation?

Mostly tagging, but also pointing folks to a related issue #1512194: Use HTML5 datalists for autocomplete

What's the status on this issue?

I'm about to implement this attribute in core:
#1955282: Disable Autocapitalize and Autocorrect on user login forms

#24: autocomplete-1275764-24.patch queued for re-testing.

Well, the function theme_textfield() & theme_password() have disappeared as have the functional tests. Leaving all we've got to re-roll the patch from #24 being this.

And that doesn't address @sun's concerns either....

#31: autocomplete-1275764-31.patch queued for re-testing.

31: autocomplete-1275764-31.patch queued for re-testing.

Patch failed on SimplyTest.me too. @jibran or @amontero want to roll a new one?

Just a re-roll... Hope it passes now. Still don't see the expected output though.

Better issue title. Still not sure about this.

#autocomplete would be ambiguous with the existing #autocomplete_path, which has a related, but very different meaning.

It would make more sense to me if we'd design an actual "API" around the #autocomplete attribute - instead of only copying values from A to B.

Thanks @sun - I was going through old issues about HTML5 for a presentation I'm about to give. Thought I'd nudge this ahead, if only to get a patch that still applies. The API around #autocomplete sounds potentially quite useful.

autocomplete

1. +++ b/core/includes/form.inc
   @@ -41,7 +41,7 @@ function drupal_form_submit($form_arg, FormStateInterface $form_state) {
   - *     Properties used: #title, #value, #options, #description, #extra,
   + *     Properties used: #action, #method, #attributes, #children, #autocomplete
     *     #multiple, #required, #name, #attributes, #size.
   

   It seems like this would just add #autocomplete, not sure why it removes others and duplicates #attributes.

2. +++ b/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php
   @@ -59,6 +60,7 @@ public static function processPasswordConfirm(&$element, FormStateInterface $for
   +      '#autocomplete' => empty($element['#autocomplete']) ? NULL : $element['#autocomplete'],  ¶
   

   Trailing whitespace here

This needs some automated testing.

needs work for #50. 8.1 because its a feature

Also why not use the existing API for attributes?

$element['#attributes']['autocomplete'] = 'on';
$element['#attributes']['autocomplete'] = 'off';

Thank you for sharing your idea for improving Drupal.

We are working to decide if this proposal meets the Criteria for evaluating proposed changes. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or there is no community support. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

wanted to bump 1 more time before closing as this had a lot of traffic at the time (8 years ago)