Download & Extend

Autocomplete errors on whitespace only

Project:Drupal core
Version:6.x-dev
Component:javascript
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs review
Issue tags:drupalconparis2009, drupalconsprint

Issue Summary

Put a single space in an autocomplete field and you get "An HTTP error 403 occured".

I Presume we need some sort of JS equivalent of trim() on the text that gets sent to autocomplete.

Comments

#1

Version:6.x-dev» 5.1

I get this error as well but i get it in D5.1

i created a text field that receives autocompletion in my custom form (fapi) and when i try to pass any number of whitespace characters into the field i get a 403 error. my custom autocomplete function is never called for the return values.

#2

Version:5.1» 6.x-dev

the regular way is that bugfixes go in the current dev version, and then are backported

#3

Status:active» closed (won't fix)

I get none of this on today's HEAD.

#4

This error still exist, but I made a solution on my copy.

on misc/autocomplete.js file I added a condition to not allow space(s) only searchString.

if (this.input.value.replace(new RegExp("^[\\s]+", "g"), "")) {
    this.db.search(this.input.value);
}

#5

Version:6.x-dev» 7.x-dev
Status:closed (won't fix)» active

I just tested this in 7.x, and it is still an issue.

#6

Rather, it seems there may have been a regression as this doesn't occur in 6.13

#7

Status:active» needs review

I think it might also depend on what the autocomplete is feeding off i.e. whether it's a taxonomy autocomplete or whatever. So sometimes you won't see this bug.

I can confirm taxonomy autocomplete in HEAD throws this error. The solution is to detect a whitespace-only submission and return an empty array. I think this isn't working as designed: it's just confusing for the user who doesn't know why they're getting a popup alert.

Patch attached for taxonomy alert. @BrightLoudNoise, can you confirm anywhere else you're seeing this?

AttachmentSizeStatusTest resultOperations
taxonomy_autocomplete_whitespace.patch.txt874 bytesIgnoredNoneNone

#8

Thanks JP

I tested the patch, and it has indeed corrected the problem on Article nodes.

Taxonomy term fields also suffer from this problem, should I split that off into another issue? or expand this a more general autocomplete bugfix?

#9

BrightLoudNoise, I don't know where we go from here. We need more reviews of this patch to push it into core. Help, anyone?

Regarding taxonomy term fields, I'll have a look at it. Is this field API i.e. taxonomy fields on node like CCK fields in D7? I'm not hot on field API but I'll see what I can do.

#10

taxonomy_autocomplete was renamed to taxonomy_autocomplete_legacy as part of #526122: Autocomplete widget for taxonomy term fields, and should be considered deprecated once the canned article content-type is updated to use taxonomy term fields.

I've copied and pasted your whitespace fix into the taxonomy_autocomplete function as well, tested it on my local install.

patch is attached.

AttachmentSizeStatusTest resultOperations
taxonomy_autocomplete_whitespace2.patch1.45 KBIdlePassed: 12708 passes, 0 fails, 0 exceptionsView details | Re-test

#11

Status:needs review» needs work

Proper punctuation is missing, the phrasing also seems non-core-ish to me. And most importantly, we never put more than 1 statement per line in Drupal core. Finally, I'm fairly sure we always call the exit language construct (it's not a function) without parameters, so you should remove the parentheses.

#12

Corrections made based on comments in #11, Thanks for the review Wim!

AttachmentSizeStatusTest resultOperations
taxonomy_autocomplete_whitespace3.patch1.46 KBIdlePassed: 12744 passes, 0 fails, 0 exceptionsView details | Re-test

#13

Status:needs work» needs review

#14

It seems to me that taxonomy is one area in which this issue may occur, but there are likely others. I believe the problem should be fixed in the autocomplete.js code so we only have to fix it once and those implementing autocomplete in a module don't have to worry about this case.

AttachmentSizeStatusTest resultOperations
autocomplete.patch670 bytesIdlePassed: 12805 passes, 0 fails, 0 exceptionsView details | Re-test

#15

Status:needs review» reviewed & tested by the community
Issue tags:+drupalconsprint

Nice one. This is a neater solution, but someone closer to core than me should confirm it's OK: maybe it looks like being forgiving of slightly sloppy code.

Strings get trimmed by autocomplete.js anyway, so that should mean that it's meant to ignore whitespace. This I think makes it more consistent.

Confirmed it works on D7 head for both the issues raised here, anyway.

#16

Status:reviewed & tested by the community» fixed

Cool. Committed to HEAD. :)

#17

#515262: Autocomplete requires ARIA for screen-reader users is a partial duplicate of this issue.

#18

Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

#19

Version:7.x-dev» 6.x-dev
Status:closed (fixed)» needs review

This bug also affects D6, so here is a patch backported from D7. This patch might conflict with the one posted in #625170: Autocomplete popup position but I would be happy to reroll either of these if the other is commited first.

Also, please tell me if this is not the appropiate place for this patch and a new issue should be opened instead.

AttachmentSizeStatusTest resultOperations
159762_autocomplete-whitespace-only-D6.patch663 bytesIgnoredNoneNone

#20

Proper D6 patch.

AttachmentSizeStatusTest resultOperations
159762_autocomplete-whitespace-only-D6.patch630 bytesIgnoredNoneNone

#21

Status:needs review» postponed (maintainer needs more info)

Please resubmit your patch with the proper filename. http://drupal.org/node/1054616

#22

Priority:critical» normal
Status:postponed (maintainer needs more info)» needs review

Are you kidding me? What's the filename got to do with it? There are patches submitted with all kinds of filenames :)

#23

Just try it. That is how the new testbot knows it is a patch. Otherwise it will get ignored like #20.

#24

It IS supposed to get ignored, this is a D6 patch and there's no testing for D6 patches.

#25

Not entirely true from my understanding. If it is a bug report (and not a feature request) the patch can be accepted into the D6 codebase.

#26

It is a bug report and of course it can be accepted in D6, all I'm saying is that D6 does not have automated tests so there's no need to send this patch for a testbot review.

#27

I cannot reproduce this bug in 6.x. Has it been fixed by some other change in the meantime?

nobody click here