#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

Files: 
CommentFileSizeAuthor
#10 core-autocomplete-sugg.patch724 bytesnod_
PASSED: [[SimpleTest]]: [MySQL] 63,688 pass(es). View

Comments

zyxware’s picture

Status: Active » Needs review
FileSize
674 bytes
PASSED: [[SimpleTest]]: [MySQL] 63,666 pass(es). View

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

Status: Needs work » Needs review
amateescu’s picture

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

FileSize
647 bytes
PASSED: [[SimpleTest]]: [MySQL] 63,414 pass(es). View

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

droplet’s picture

FileSize
674 bytes
PASSED: [[SimpleTest]]: [MySQL] 63,353 pass(es). View

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

Status: Needs work » Needs review
nod_’s picture

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

Status: Needs work » Needs review
FileSize
724 bytes
PASSED: [[SimpleTest]]: [MySQL] 63,688 pass(es). View
nod_’s picture

Assigned: nod_ » Unassigned
Issue tags: +ie8
amateescu’s picture

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

droplet’s picture

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

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.