Notices in taxonomy_autocomplete()

sub

Patch attached.

Is this still an issue in D8 after #93854: Allow autocompletion requests to include slashes has been fixed?

Swentel: The $matches array is never used, only $term_matches is - so you can simplify the code further by removing $matches..

You can see my patch in the more-or-less duplicate issue #665420: taxonomy autocomplete throws notice on non match

The patch here also needs to be rerolled for the core/ change at least, and we should add a test.

Tagging as novice for two tasks:

• Reroll the patch and remove the unused $matches variable as suggested above. (See the patch in #665420: taxonomy autocomplete throws notice on non match.)
• Add an automated test. (The test can probably be a simple matter of visiting the path and ensuring that there are no notices.) The test should fail when applied on its own, and pass when combined with the patch here. You can upload both the tests-only and combined patches in the issue to expose the test failure for reviewers.

I will take a stab at this, give me some time to figure out how to write the test for this.

@nanotube -- Are you still looking into this issue? If not we'll unassign it for someone else to take a crack at it. Thanks!

I'll take it this weekend.

Thanks swentel. Since this is your patch to begin with that make sense. In general though, could you give the assignee maybe a day or to two reply, just in case they're about to post something? (Especially for novice-tagged issues.) Thanks!

Two patches, first one with test alone, this will fail. Second one with fix, should pass.

We need to document the new optional variable:

 * @param $field_name
 *   The name of the term reference field.

Should be:

 * @param $field_name
 *   (optional) The name of the term reference field.

I'm not sure about that, since without a field_name this function simply does nothing. The patch simply makes sure no notices are triggered, let's get some more opinions on this.

Yes, you're right.

I'm not sure about that, since without a field_name this function simply does nothing. The patch simply makes sure no notices are triggered, let's get some more opinions on this.

Hmm, good point. In that case we should document that fact in the parameter documentation, I think.

Thanks swentel! I got tied up. I thought the test result was supposed to be "[ ]", never got it to pass on my end.

Re-rolled for changes to tests in D8.
And to bump the issue.

#16: drupal-taxonomy-autocomplete-notices-combined-1242602-16.patch queued for re-testing.

This isn't relevant anymore for D8

Thanks @swentel! Backporting the patch from #16 for Drupal 7 would be a great novice task.

Edit: Instructions here.

Attached backport patch for D7.

#21: drupal-taxonomy-autocomplete-notices-combined-1242602-21.patch queued for re-testing.

Go with this! It's only a notice removal!

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/054c8ab

This also has the "needs backport to D6" tag, so if someone wants to reopen it for backport please feel free. (However, it's my impression that non-security-related patches have not been committed to Drupal 6 for a long time.)