Autocomplete is broken (its JS is not loaded)

I'm curious, do you have a HTTP query going on for the autocompletion or does it even not do that?

I'm pretty sure this would be solved with #2535118: Toolbar subtrees not working on admin pages due to lack of theme negotiation on Toolbar's custom JSONP route

Yes, based on #3, I suspect the root cause is simply that a JS error occurred, and that no further JS executes.

To verify: please disable Toolbar module and test again.

There are no JS errors.

Just go to node/add/article and try to use the author autocomplete.

No errors, no HTTP requests, no visual feedback. *Nothing* happens.

Which, isn't very surprising, considering that autocomplete.js. is. not. even. loaded :)

Which, isn't very surprising, considering that autocomplete.js. is. not. even. loaded :)

Given this, I don't believe this could be related to #2535118: Toolbar subtrees not working on admin pages due to lack of theme negotiation on Toolbar's custom JSONP route at all.

EDIT: also: confirmed what Berdir said in manual testing.

Ok, found and fixed *two* bugs.

1) merge wasn't used properly. This didn't actually result in a visible bug.
2) the attachment was added to $element after creating $metadata from it and then re-applying it dropped it again.

This was definitely caused by the bubbleable links issue, because that switched to BubbleableMetadata, before it was just Cacheable, without #attachments.

Tests should be easy, just need to make sure that autocomplete.js is loaded.

+1 for every word in #8.

This has to be critical no? We could not ship Drupal 8 with this bug. Not being able to change the author of a node is a critical bug.

+1

Heh I was about to do the same thing but couldn't quite decide. Autocomplete not autocompleting is in the same category as page titles disappearing, so yes agreed on critical even if the bug itself is a straightforward one.

CNW for tests, else RTBC

+++ b/core/modules/system/src/Tests/Form/ElementTest.php
@@ -151,6 +151,9 @@ public function testFormAutocomplete() {
+    // Make sure that the autocomplete library is added.
+    $this->assertRaw('core/misc/autocomplete.js');
+

I was wondering whether this is the right way to ensure that. Yes it works, but it will break once, for some probably valid reason, you enable js aggregation for tests as well

We agreed on IRC that this would be a follow up.

Committed f678e37 and pushed to 8.0.x. Thanks!

+++ b/core/lib/Drupal/Core/Render/Element/FormElement.php
@@ -128,10 +128,10 @@ public static function processAutocomplete(&$element, FormStateInterface $form_s
-        $metadata->merge($url);
+        $metadata = $metadata->merge($url);

We should add a test for this in a followup. From @Wim Leers in IRC:

you could create an autocomplete route with a custom cache tag, and then verify that that cache tag is present in the response

Too late for the commit message, but whatever :)

Oops!

Just hit this working on my 1st D8 site build, always nice when not only is the issue already written, but it's fixed too! Thanks @Berdir and @mbovan!

So a pretty satisfactory git pull from last 24 hours, overall :)