Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#20 | 159762_autocomplete-whitespace-only-D6.patch | 630 bytes | amateescu |
#19 | 159762_autocomplete-whitespace-only-D6.patch | 663 bytes | amateescu |
#14 | autocomplete.patch | 670 bytes | paul.lovvik |
#12 | taxonomy_autocomplete_whitespace3.patch | 1.46 KB | BrightLoudNoise |
#10 | taxonomy_autocomplete_whitespace2.patch | 1.45 KB | BrightLoudNoise |
Comments
Comment #1
litwol CreditAttribution: litwol commentedI 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.
Comment #2
yched CreditAttribution: yched commentedthe regular way is that bugfixes go in the current dev version, and then are backported
Comment #3
chx CreditAttribution: chx commentedI get none of this on today's HEAD.
Comment #4
mello.capinpin CreditAttribution: mello.capinpin commentedThis 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.
Comment #5
BrightLoudNoise CreditAttribution: BrightLoudNoise commentedI just tested this in 7.x, and it is still an issue.
Comment #6
BrightLoudNoise CreditAttribution: BrightLoudNoise commentedRather, it seems there may have been a regression as this doesn't occur in 6.13
Comment #7
jp.stacey CreditAttribution: jp.stacey commentedI 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?
Comment #8
BrightLoudNoise CreditAttribution: BrightLoudNoise commentedThanks 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?
Comment #9
jp.stacey CreditAttribution: jp.stacey commentedBrightLoudNoise, 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.
Comment #10
BrightLoudNoise CreditAttribution: BrightLoudNoise commentedtaxonomy_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.
Comment #11
Wim LeersProper 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.
Comment #12
BrightLoudNoise CreditAttribution: BrightLoudNoise commentedCorrections made based on comments in #11, Thanks for the review Wim!
Comment #13
lilou CreditAttribution: lilou commentedComment #14
paul.lovvik CreditAttribution: paul.lovvik commentedIt 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.
Comment #15
jp.stacey CreditAttribution: jp.stacey commentedNice 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.
Comment #16
webchickCool. Committed to HEAD. :)
Comment #17
Everett Zufelt CreditAttribution: Everett Zufelt commented#515262: Autocomplete requires ARIA for screen-reader users is a partial duplicate of this issue.
Comment #19
amateescu CreditAttribution: amateescu commentedThis 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.
Comment #20
amateescu CreditAttribution: amateescu commentedProper D6 patch.
Comment #21
dpearcefl CreditAttribution: dpearcefl commentedPlease resubmit your patch with the proper filename. http://drupal.org/node/1054616
Comment #22
amateescu CreditAttribution: amateescu commentedAre you kidding me? What's the filename got to do with it? There are patches submitted with all kinds of filenames :)
Comment #23
dpearcefl CreditAttribution: dpearcefl commentedJust try it. That is how the new testbot knows it is a patch. Otherwise it will get ignored like #20.
Comment #24
amateescu CreditAttribution: amateescu commentedIt IS supposed to get ignored, this is a D6 patch and there's no testing for D6 patches.
Comment #25
dpearcefl CreditAttribution: dpearcefl commentedNot 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.
Comment #26
amateescu CreditAttribution: amateescu commentedIt 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.
Comment #27
c960657 CreditAttribution: c960657 commentedI cannot reproduce this bug in 6.x. Has it been fixed by some other change in the meantime?