starred/cypht

Fork 0

mirror of https://github.com/cypht-org/cypht.git synced 2026-04-25 04:56:03 +03:00

[GH-ISSUE #18] minor: address autocompletion is too strict #13

New issue

Closed

opened 2026-02-25 21:33:55 +03:00 by kerem · 9 comments

kerem commented

2026-02-25 21:33:55 +03:00

Owner

Originally created by @dumblob on GitHub (Aug 20, 2015).
Original GitHub issue: https://github.com/cypht-org/cypht/issues/18

Originally assigned to: @jasonmunro on GitHub.

In modules/contacts/site.js autocomplete_contact() there is a check for minimal length of the requested string. 4 is used as minimum, which is in my opinion not necessary as in practice we're interested in every single character, because of emails of very short length and very long length a@x.y b@x.y c@x.y d@x.y Andreas Withe Super Extra Long German Name <andreaswithsuperextralonggermanname@superextralongdomainbecausewecan.com> being next to each other. I wouldn't worry about the overhead introduced by changing the minimum to 1 as nobody types thousands of requests per second (so AJAX and PHP would be fine) and regarding the backend DB, it's suited for such load.

Also, I wonder if contact_suggestions[i] in autocomplete_contact_results() shouldn't be converted to HTML tag attribute compatible string as according to https://tools.ietf.org/html/rfc2822#section-3.4 , the domain part can contain really interesting characters potentially invalid in attribute strings (non white space controls, character references, ", ' to name a few). I myself wouldn't trust at all that the DB content is clean ;)

Originally created by @dumblob on GitHub (Aug 20, 2015). Original GitHub issue: https://github.com/cypht-org/cypht/issues/18 Originally assigned to: @jasonmunro on GitHub. In `modules/contacts/site.js` `autocomplete_contact()` there is a check for minimal length of the requested string. `4` is used as minimum, which is in my opinion not necessary as in practice we're interested in every single character, because of emails of very short length and very long length `a@x.y` `b@x.y` `c@x.y` `d@x.y` `Andreas Withe Super Extra Long German Name <andreaswithsuperextralonggermanname@superextralongdomainbecausewecan.com>` being next to each other. I wouldn't worry about the overhead introduced by changing the minimum to `1` as nobody types thousands of requests per second (so AJAX and PHP would be fine) and regarding the backend DB, it's suited for such load. Also, I wonder if `contact_suggestions[i]` in `autocomplete_contact_results()` shouldn't be converted to HTML tag attribute compatible string as according to https://tools.ietf.org/html/rfc2822#section-3.4 , the domain part can contain really interesting characters potentially invalid in attribute strings (non white space controls, character references, `"`, `'` to name a few). I myself wouldn't trust at all that the DB content is clean ;)

kerem

2026-02-25 21:33:55 +03:00

closed this issue
added the
bug
label

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@jasonmunro commented on GitHub (Aug 25, 2015):

This is still a work in progress and needs some love. I was trying to leverage native browser auto-completing via the datalist tag which requires the values to be in a tag attribute. There are a number of things I don't like about this approach, so ultimately we may need to roll our own solution here. WRT the content not being clean, you are absolutely right, thanks for catching that. I got lazy and output the suggestions directly from a handler mod without filtering them through an output mod. Regardless of whether we stick with native auto-complete or build a custom UI component for this, I will get that part fixed up.

@jasonmunro commented on GitHub (Aug 25, 2015): This is still a work in progress and needs some love. I was trying to leverage native browser auto-completing via the datalist tag which requires the values to be in a tag attribute. There are a number of things I don't like about this approach, so ultimately we may need to roll our own solution here. WRT the content not being clean, you are absolutely right, thanks for catching that. I got lazy and output the suggestions directly from a handler mod without filtering them through an output mod. Regardless of whether we stick with native auto-complete or build a custom UI component for this, I will get that part fixed up.

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@dumblob commented on GitHub (Aug 25, 2015):

I was trying to leverage native browser auto-completing via the datalist tag which requires the values to be in a tag attribute. There are a number of things I don't like about this approach, so ultimately we may need to roll our own solution here.

IMHO it's not such a bad idea (but I'm not a web expert). In general I can think of only one minor concern, namely the fact, that auto-completion is browser-dependent, so the behavior might differ. But one can consider this as both positive and negative thing. I myself consider it as a more positive one.

You surely had something else in mind though, what are those "number of things" you don't like on this approach?

@dumblob commented on GitHub (Aug 25, 2015): > I was trying to leverage native browser auto-completing via the datalist tag which requires the values to be in a tag attribute. There are a number of things I don't like about this approach, so ultimately we may need to roll our own solution here. IMHO it's not such a bad idea (but I'm not a web expert). In general I can think of only one minor concern, namely the fact, that auto-completion is browser-dependent, so the behavior might differ. But one can consider this as both positive and negative thing. I myself consider it as a more positive one. You surely had something else in mind though, what are those "number of things" you don't like on this approach?

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@jasonmunro commented on GitHub (Aug 28, 2015):

The way this works is we use an ajax call to populate a list of available auto-complete values - but the browser still filters that list against what is in the text box from left to right. This makes auto-completing the second or third comma separated address problematic. I also want to bring in suggestions that match both the E-mail or the name associated with it. Again the way this works means those suggestions will not be shown after the left-right comparison (this is all with chromium, so other browsers may differ - another potential issue).

@jasonmunro commented on GitHub (Aug 28, 2015): The way this works is we use an ajax call to populate a list of available auto-complete values - but the browser still filters that list against what is in the text box from left to right. This makes auto-completing the second or third comma separated address problematic. I also want to bring in suggestions that match both the E-mail or the name associated with it. Again the way this works means those suggestions will not be shown after the left-right comparison (this is all with chromium, so other browsers may differ - another potential issue).

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@dumblob commented on GitHub (Aug 29, 2015):

It's true, that the double filtering might cause issues. Do you have any experience with long datalists (possibly more than thousand contacts) in the DOM tree? I was just thinking about on-demand incremental fetching of the whole contact list based on the typed characters in the field (which is pretty much what the current implementation does, except that it removes already fetched datalist option tags).

I also want to bring in suggestions that match both the E-mail or the name associated with it.

IIRC the RFC correctly, the full name and email address has format "Some Name" <email@addr.domain>. I tried this format in Firefox 39 on http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_datalist and it worked (I put there two different option tags: <option value""some name" <abc@def.xyz>"> <option value""abc def" <some@name.xyz>"> and tried matching def and some etc.). Or did you mean something else by "match both"?

On the other hand, matching also other "hidden" properties (like day of birth or some plain description) from the address/contact list might also be a plus and that won't work at all with the datalist tag as there is no easy solution how to "hide" the additional information in the value attribute.

@dumblob commented on GitHub (Aug 29, 2015): It's true, that the double filtering might cause issues. Do you have any experience with long datalists (possibly more than thousand contacts) in the DOM tree? I was just thinking about on-demand incremental fetching of the whole contact list based on the typed characters in the field (which is pretty much what the current implementation does, except that it removes already fetched datalist option tags). > I also want to bring in suggestions that match both the E-mail or the name associated with it. IIRC the RFC correctly, the full name and email address has format `"Some Name" <email@addr.domain>`. I tried this format in Firefox 39 on http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_datalist and it worked (I put there two different option tags: `<option value""some name" <abc@def.xyz>">` `<option value""abc def" <some@name.xyz>">` and tried matching `def` and `some` etc.). Or did you mean something else by "match both"? On the other hand, matching also other "hidden" properties (like day of birth or some plain description) from the address/contact list might also be a plus and that won't work at all with the datalist tag as there is no easy solution how to "hide" the additional information in the value attribute.

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@jasonmunro commented on GitHub (Sep 2, 2015):

Or did you mean something else by "match both"?

In Chromium the only entries that make it past the browser filter are those that exactly match from left to right, including things like quotes. I just tested FF and confirmed what you are seeing, which is better, but still will have issues as you mentioned matching on other fields, and with multiple addresses per field.

Also, I fixed the contact suggestion filtering you mentioned in the OP in github.com/jasonmunro/hm3@26fd9fe3cf

Thanks again for all the great feedback!

@jasonmunro commented on GitHub (Sep 2, 2015): > Or did you mean something else by "match both"? In Chromium the only entries that make it past the browser filter are those that exactly match from left to right, including things like quotes. I just tested FF and confirmed what you are seeing, which is better, but still will have issues as you mentioned matching on other fields, and with multiple addresses per field. Also, I fixed the contact suggestion filtering you mentioned in the OP in https://github.com/jasonmunro/hm3/commit/26fd9fe3cf9e778a7e0639ec36cc894ea0a5401b Thanks again for all the great feedback!

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@jasonmunro commented on GitHub (Sep 10, 2015):

I committed a new auto-complete system that does not use native browser completion. It's pretty slick I think, but I have only tested it in chromium. It needs more attention and clean-up but it's pretty cool so far.

@jasonmunro commented on GitHub (Sep 10, 2015): I committed a new auto-complete system that does not use native browser completion. It's pretty slick I think, but I have only tested it in chromium. It needs more attention and clean-up but it's pretty cool so far.

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@jasonmunro commented on GitHub (Sep 14, 2015):

I'm going to close this since the new system addresses the points made here. If you have any additional problems with the contact auto-complete feature, please file another issue. Thanks!

@jasonmunro commented on GitHub (Sep 14, 2015): I'm going to close this since the new system addresses the points made here. If you have any additional problems with the contact auto-complete feature, please file another issue. Thanks!

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@jasonmunro commented on GitHub (Dec 8, 2015):

forgot about that bit. Just updated the code to allow auto-complete to kick in if the field value length is greater than 0. Thanks for the reminder!

@jasonmunro commented on GitHub (Dec 8, 2015): forgot about that bit. Just updated the code to allow auto-complete to kick in if the field value length is greater than 0. Thanks for the reminder!

kerem commented

2026-02-25 21:33:55 +03:00

Author

Owner

@dumblob commented on GitHub (Dec 8, 2015):

Awesome. You're welcome.

@dumblob commented on GitHub (Dec 8, 2015): Awesome. You're welcome.