Form API autocomplete is broken for routes

We should at least have a @todo to replace menu_item_route_access() with the new method at #2046737: Add a method to the AccessManager that only needs a route name and parameters

@@ -4087,7 +4087,14 @@ function theme_vertical_tabs($variables) {
+  if (!empty($element['#autocomplete_route'])) {
+    $route = Drupal::service('router.route_provider')->getRouteByName($element['#autocomplete_route']);
+    $element['#autocomplete_path'] = trim($route->getPath(), '/');
+    $map = array();

Yeah you don't want to use the router provider by the url generator and pass in the route name and paramters.

Working on the patch 10mins, getting the tests right: 30 minutes. Good night!

I have chosen to use route_name to just tell people that it is the name.
This for sure needs some documentation, though it is seriously too late now.

Working on the patch 10mins, getting the tests right: 30 minutes. Good night!

I have chosen to use route_name to just tell people that it is the name.
This for sure needs some documentation, though it is seriously too late now.

Postgresql catches this hard as user entity tries to load the uid 'autocomplete', which fails strict typing. Mysql driver probably ignores or munges the data.

Thank you for the review!

Here is a test for the author form. Yes we could invent even more tests for every damn place a user auto-completion appears but I don't think we need that as we test the same functionality all over again.

A general question is whether we want to support the new routing system in drupal_valid_url, see https://drupal.org/node/876580 for a rough comment.

+++ b/core/includes/form.incundefined
+++ b/core/includes/form.incundefined
@@ -4078,16 +4078,42 @@ function theme_vertical_tabs($variables) {
  * @code
  * '#autocomplete_path' => 'mymodule_autocomplete/' . $some_key . '/' . $some_id,
  * @endcode
+ * Or if your route is called 'mymodule_autocomplete' with the path of
+ * '/mymodule-autocomplete/{a}/{b}':
+ * @code
+ * '#autocomplete_route_name' => 'mymodule_autocomplete',
+ * '#autocomplete_route_parameters' => array('a' => $some_key, 'b' => $some_id),
+ * @endcode
  * The user types in "keywords" so the full path called is:
  * 'mymodule_autocomplete/$some_key/$some_id?q=keywords'
  *
- * @param $element
+ * @param array $element
  *   The form element to process. Properties used:
  *   - #autocomplete_path: A system path to be used as callback URL by the
  *     autocomplete JavaScript library.
+ *   - #autocomplete_route_name: A route to be used as callback URL by the
+ *     autocomplete JavaScript library.
+ *   - #autocomplete_route_parameters: The parameters to be used in conjunction
+ *     with the route name.
+ * @param array $form_state
+ *   An associative array containing the current state of the form.
+ *
+ * @return array
+ *   The form element.

Just a note: It may be nice for new developers to point out why one would use path or route. I think this could be done initially in a change notice and in handbook documentation.

On the longrun you should never use autocomplete_path, that is just in there as long we haven't converted all autocompletions.

I think this approach makes sense, but I'm not crazy about:

#autocomplete_path is left for hook_menu() page callbacks

I'd rather see this issue remove it, so that entity_reference module can be updated to use #autocomplete_route in all cases. Tagging for upgrade path because of that. Looks like #1987858: Convert taxonomy_autocomplete() to a new style controller is close to done. Can we focus on getting that in quickly, and then do the full conversion of #autocomplete_* here?

@alexpott approves, since this is a critical fallout of router conversion.

I agree. There is neither a fundamental problem nor actually a lot of work if we keep autocomplete_path in for now. This seems really like overthinking ;)

Remove wrong tag.

looks great

If that is the last usage of #autocomplete_path, why not put this in now, and then remove #autocomplete_path over there.

Well, per #20, putting this in now only fixes the user autocompletes that the patch touches, and leaves ER fields targeting users still using the broken #autocomplete_path. But I guess that's not a further regression, just a very incomplete fix. So ok, let's do this one first. As long as we recognize that this is in fact an API change, not merely an addition, because the retaining of #autocomplete_path is just an artifact of patch order, not something that's releasable.

+++ b/core/includes/form.inc
@@ -4078,16 +4078,42 @@ function theme_vertical_tabs($variables) {
+    $path = Drupal::urlGenerator()->generate($element['#autocomplete_route_name'], $parameters);
...
+++ b/core/includes/form.inc
@@ -4078,16 +4078,42 @@ function theme_vertical_tabs($variables) {
+    $path = url($element['#autocomplete_path'], array('absolute' => TRUE));

I don't know why we need the second one to be an absolute URL, but I think that's been that way in HEAD since autocomplete functionality was first added to Drupal. Given that, we should probably be consistent and make the new way also output absolute URLs.

#26 was an xpost with #25. I'm ok with it being a follow up though, so back to RTBC.

Committed/pushed to 8.x, thanks!

Will need a change notice.

https://drupal.org/node/2070985

Added #2071115: Remove #autocomplete_path in favor of #autocomplete_route for #22.

Good addition tim!

#2651798: claroAutocompleteTest passes, but log shows a 404