Autocomplete errors on whitespace only

jakeg - July 16, 2007 - 15:56
Project:Drupal
Version:7.x-dev
Component:javascript
Category:bug report
Priority:critical
Assigned:Unassigned
Status:closed
Issue tags:drupalconparis2009, drupalconsprint
Description

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.

#1

litwol - July 16, 2007 - 22:38
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

yched - July 16, 2007 - 22:47
Version:5.1» 6.x-dev

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

#3

chx - September 2, 2007 - 05:31
Status:active» won't fix

I get none of this on today's HEAD.

#4

mello.capinpin - August 19, 2008 - 06:15

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

BrightLoudNoise - September 1, 2009 - 00:43
Version:6.x-dev» 7.x-dev
Status:won't fix» active

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

#6

BrightLoudNoise - September 1, 2009 - 00:44

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

#7

jp.stacey - September 1, 2009 - 09:35
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

BrightLoudNoise - September 2, 2009 - 17:15

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

jp.stacey - September 3, 2009 - 12:25

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

BrightLoudNoise - September 3, 2009 - 17:05

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

Wim Leers - September 4, 2009 - 15:23
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

BrightLoudNoise - September 4, 2009 - 15:43

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

lilou - September 4, 2009 - 19:11
Status:needs work» needs review

#14

paul.lovvik - September 5, 2009 - 09:33

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

jp.stacey - September 5, 2009 - 11: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

webchick - September 5, 2009 - 12:03
Status:reviewed & tested by the community» fixed

Cool. Committed to HEAD. :)

#17

Everett Zufelt - September 6, 2009 - 13:37

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

#18

System Message - September 20, 2009 - 13:40
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.