#675446-238:
There appears to a major regression in the committed code: duplicates are suggested!

Steps to reproduce:

  1. Make sure a free tagging field already has the "foo" and "bar" values. Assume these are the only two terms in the system.
  2. Now go back to the node and edit, type a comma and "b".
  3. Expected: nothing is suggested. Reality: "bar" is suggested again.

This problem was in fact implicitly being demonstrated in #230.

duplicates

CommentFileSizeAuthor
#49 interdiff-2186647-46-49.txt590 bytesacbramley
#49 2186647-49.patch1019 bytesacbramley
#47 interdiff_43_46.txt463 bytesrishabh vishwakarma
#46 2186647-46.patch1.05 KBrishabh vishwakarma
#43 interdiff_41-43.txt882 bytesnitin shrivastava
#43 2186647-27.patch1.05 KBnitin shrivastava
#41 2186647-41.patch828 bytesrishabh vishwakarma
#35 2186647--after--patch--pic.png19.61 KBvikashsoni
#35 2186647--before--patch--pic.png26.74 KBvikashsoni
#31 2186647-before_patch-27.png12.33 KBabhijith s
#31 2186647-after_patch-27.png17.56 KBabhijith s
#28 beforepatch.jpg205.92 KBranjith_kumar_k_u
#28 afterpatch.jpg225.64 KBranjith_kumar_k_u
#27 interdiff_25_27.txt1.18 KBanmolgoyal74
#27 2186647-27.patch1.61 KBanmolgoyal74
#25 2186647-25.patch1.58 KBrajneeshb
#23 core-autocomplete-sugg-2186647-23.patch922 bytesrahul.shinde
#10 core-autocomplete-sugg.patch724 bytesnod_
#6 core-js-ui-autocomplete-2186647-6.patch674 bytesdroplet
#5 core-js-ui-autocomplete-2186647-5.patch647 bytesdroplet
#1 core-js-ui-autocomplete-2186647-1.patch674 byteszyxware

Issue fork drupal-2186647

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

zyxware’s picture

zyxware commented
Status: Active » Needs review
StatusFileSize
new core-js-ui-autocomplete-2186647-1.patch674 bytes

The data returned has label and values as key value pairs as an array of objects and showSuggestions was still searching for values in an array. The attached patch checks for existing terms in the value property of the objects returned and splices such objects from the array.

With the patch applied I see an anomaly in how the logic works.

Suppose the field already has bar in the value (with bar and baz as valid items in the term database) and I type in 'ba' the autocomplete comes up with 'baz' and no 'bar'. This is alright. I then pick 'baz' and type 'ba' again, no autocomplete comes up as both 'bar' and 'baz' are in the line. Now I backspace and remove both 'bar' and 'baz' from the box. I type 'ba' again and don't get anything. The suggestions for 'ba' are cached locally and not recalculated locally again. Not sure if that is the right way to do. Shouldn't just the original return values from the server be cached and recalculations always done against current items in the text box?

Status: Needs review » Needs work

The last submitted patch, 1: core-js-ui-autocomplete-2186647-1.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review

1: core-js-ui-autocomplete-2186647-1.patch queued for re-testing.

amateescu’s picture

amateescu commented
Assigned: Unassigned » nod_

Re #1: Yes, it seems that the local cache for autocomplete suggestions is a real problem. Talked about it with @nod_ in IRC and he said it's okay to remove it, but let's get an official response here as well :)

droplet’s picture

droplet commented
StatusFileSize
new core-js-ui-autocomplete-2186647-5.patch647 bytes

It's all we have to do. Use jQuery UI function, easy life :) (I'm confusing myself, skip this patch, thanks)

droplet’s picture

droplet commented
StatusFileSize
new core-js-ui-autocomplete-2186647-6.patch674 bytes

Reroll patch #1

Status: Needs review » Needs work

The last submitted patch, 6: core-js-ui-autocomplete-2186647-6.patch, failed testing.

droplet’s picture

droplet commented
Status: Needs work » Needs review

6: core-js-ui-autocomplete-2186647-6.patch queued for re-testing.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Needs work

looping over an array that changes it's length is dodgy.

We can do the same without messing with the length and splice.

var filteredSuggestions = suggestions.filter(function (suggestion) {
  return tagged.indexOf(suggestion.value) === -1;
});
nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Needs review
StatusFileSize
new core-autocomplete-sugg.patch724 bytes
nod_’s picture

nod_
Primary language French
Location Lille
commented
Assigned: nod_ » Unassigned
Issue tags: +ie8
amateescu’s picture

amateescu commented

I thought we agreed to also remove the client cache in this patch?

droplet’s picture

droplet commented

Both patches removed the first suggestion.

For example, you have "Kay" in the Tags terms list, You type in "Kay", no suggestions

droplet’s picture

droplet commented
Status: Needs review » Needs work

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

rahul.shinde’s picture

rahul.shinde
he/him
Primary language Marathi
Location Pune
commented
Status: Needs work » Needs review
Issue tags: -JavaScript +JavaScript
StatusFileSize
new core-autocomplete-sugg-2186647-23.patch922 bytes

I have re-rolled the patch.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 8.9.x-dev » 9.1.x-dev
Status: Needs review » Needs work

There are better array functions these days, includes is not supported an IE so let's use .find() instead for which we already have a polyfill.

Also please check out this page: https://www.drupal.org/docs/frontend-developer-tools-for-drupal-core you should not edit directly the .js file. The workflow is to edit the *.es6.js file and generate the *.js file afterwards. There is a warning on top of every JS file, check it out.

rajneeshb’s picture

rajneeshb commented
Status: Needs work » Needs review
StatusFileSize
new 2186647-25.patch1.58 KB

I have re-rolled the patch and cover the all points of #24

Status: Needs review » Needs work

The last submitted patch, 25: 2186647-25.patch, failed testing. View results

anmolgoyal74’s picture

anmolgoyal74 commented
Status: Needs work » Needs review
StatusFileSize
new 2186647-27.patch1.61 KB
new interdiff_25_27.txt1.18 KB
ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
StatusFileSize
new afterpatch.jpg225.64 KB
new beforepatch.jpg205.92 KB

The above patch works fine,it resolves the issue.
before applying the patchbefore patch
after after patch

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

tanubansal’s picture

tanubansal commented

Tested #27, working fine on 9.1
This can be moved to RTBC

abhijith s’s picture

abhijith s commented
StatusFileSize
new 2186647-after_patch-27.png17.56 KB
new 2186647-before_patch-27.png12.33 KB

The patch #27 works fine.
Including screenshots
before patch:
before
after patch:
after

RTBC

amateescu’s picture

amateescu commented
Status: Needs review » Needs work

I think #4 still needs to be addressed, right?

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

vikashsoni’s picture

vikashsoni commented
StatusFileSize
new 2186647--before--patch--pic.png26.74 KB
new 2186647--after--patch--pic.png19.61 KB

Applied #27 patch applied successfully and looks good for me
After patch not able to select duplicate entry in autocomplete
for ref sharing screenshot ....

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan’s picture

mstrelan commented
Issue tags: -ie8 +Bug Smash Initiative

I think the changes requested in #24 can be reverted now that we don't support IE anymore. As mentioned in #32 we still need to address #4.

acbramley’s picture

acbramley commented
Issue tags: +Needs tests, +Needs reroll

Confirmed the issue still exists on 10.1.x. The same solution should be able to apply, but we need tests and a reroll as well as removing the local cache.

karishmaamin’s picture

karishmaamin commented
Status: Needs work » Needs review
StatusFileSize
new 2186647-40.patch830 bytes

Re-rolled #27 patch against 10.1.x.

rishabh vishwakarma’s picture

rishabh vishwakarma commented
StatusFileSize
new 2186647-41.patch828 bytes

Adding reroll for 10.1.x. Patch #27 throws the following error -
error: patch failed: core/misc/autocomplete.js:65
error: core/misc/autocomplete.js: patch does not apply

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Still needs tests

nitin shrivastava’s picture

nitin shrivastava commented
Status: Needs work » Needs review
StatusFileSize
new 2186647-27.patch1.05 KB
new interdiff_41-43.txt882 bytes

Fix CCF errors.

nitin shrivastava’s picture

nitin shrivastava commented
Status: Needs review » Needs work
mstrelan’s picture

mstrelan commented

My suggestion in #38 was a little misguided. What I meant is that we can indeed use includes instead of find.

rishabh vishwakarma’s picture

rishabh vishwakarma commented
Status: Needs work » Needs review
StatusFileSize
new 2186647-46.patch1.05 KB

Added changes according to #45

rishabh vishwakarma’s picture

rishabh vishwakarma commented
StatusFileSize
new interdiff_43_46.txt463 bytes
acbramley’s picture

acbramley commented
Status: Needs review » Needs work

#46 doesn't fix the bug, and still needs tests.

Please guys, do not reroll or make changes to this patch if you're not going to properly test the functionality fixes the bug or read any other comments in the issue.

acbramley’s picture

acbramley commented
Issue tags: -Needs reroll
StatusFileSize
new 2186647-49.patch1019 bytes
new interdiff-2186647-46-49.txt590 bytes

Here's a working fix with .includes. Still looking for existing JS tests for this widget but I don't think there are any.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

akalam made their first commit to this issue’s fork.

akalam opened merge request !11553

akalam’s picture

akalam commented

I've created a MR with the changes of #79 to apply against D11 or D10.4. I've tested the patch locally and it works avoiding to display terms alreay applied when using an entity reference field with the widget "Autocomplete (tag style)"

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.