User Agent: Mozilla/5.0 (Windows NT 6.1; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20141027113017 Steps to reproduce: Put the attached Dummy.vcf file on the phone. Contacts->Settings->Import Contacts->memory card Actual results: Phone stuck around 5 contact import, same vcf file is properly working on other phones. Expected results: It should not stuck in between and import all the contacts.

Thanks, we recently refactor this part of the code, Jose probably will have more insights. Moving this to next sprint, if we manage to get some time will look at it.

This seems related to the encoding. Is the encoding of this file UTF8, Ashay?

I am not sure about encoding, but by default I expect it as UTF-8. I don't see any error while importing. I think its a case of missing callback, while loading file or parsing in some particular condition. Thanks

I don't think it's a UTF-8 file. I ran the Unix command file on multiple vcards files, here's what I get: > file --mime-type --mime-encoding Contacts.vcf > Contacts.vcf: text/x-vcard; charset=utf-8 > file --mime-type --mime-encoding 啊的固话你_ㄕㄩㄜ奧歐.vcf > 啊的固话你_ㄕㄩㄜ奧歐.vcf: text/x-vcard; charset=utf-8 (Don't bother the characters, I monkey typed ;) ) And now, your dummy file: > file --mime-type --mime-encoding dummy.vcf > dummy.vcf: text/x-vcard; charset=binary The problem occurs at the 5th contact, if you have a look at your contacts, you don't have any special characters until the 4th contact. I guess that's why we're stuck. What do you think Ashay? Can you generate a vcard file in UTF8, in order to be sure? Thanks!

Hi, We need to test using the current file only, because the same file works properly in android but not in FFOS. Here issue is why phone gets hang while importing. If there is a problem it should show error message or some thing similar says invalid vcf file. Even I tried converting this file to UTF-8, the number of contacts reduced to 7, So doing that will remove the contacts unnecessarily. Thanks!

I agree, we should report at least an error.

[Blocking Requested - why for this release]:

[Blocking Requested - why for this release]: As UX change and may introduce new UI/string - pushing this to 2.1

Actually for 2.1 it's also too late for new UI/strings either.

I thought I would go home wondering what happened here but I finally found the culprit. It was failing not because of encoding problems, but because the code enters a loop waiting for a closing quote when has found one first. In this case, an address had a quote in it so the code looped at the end of the line waiting to find the closing one. I added a condition to check that we are indeed in a LABEL attribute, which is the only case that uses enclosing quotes. But this is just a temporal fix, or it should be, because afaik this parser tries to understand every version of vCard at once. The exception for the quotes is only needed for vCard 4.0 and we are working with a vCard 3.0 here, which I think is the root of the bug. Anyway, the UI doesn't freeze anymore in this corner case with my patch.

Hi, Even after apply your patch in V2.0, I am able to reproduce this issue. Still the UI freezes at 5. Please check. Thanks

Oops, I saw the error in my patch. It seems I submitted an old version of my code... Will be fixed by tomorrow, sorry!

I've just updated my pull request with the correct patch. Would you mind testing it again to check if everything is fine this time? Thanks!

@Adrian, it works fine. Thanks

it seems to have a patch with right pull request so land it in master.

Comment on attachment 8516733 [details] Pull request #25812 Hi Adrian, your patch has some problems. I left a comment with the details about it in your pull request (https://github.com/mozilla-b2g/gaia/pull/25812)

Comment on attachment 8516733 [details] Pull request #25812 I spent some time trying to come up with a regex that can handle every case. You can check everything I tested here: http://regexr.com/39tt9 http://regexr.com/39tti If you see some case where this would fail, please tell me. Thanks!

Hi Adrian, Thanks for your patch. Some comments: I don't understand the change, I hope you can explain it to me. The simplicity of the previous approach was that if a quote was found, the `inLabel` variable would be switched so we always knew whether we were inside a label (or quoted parameter). Now there is a complex Regexp that operates on the whole line (for every character of the text!), but not on the actual character that we are going through anymore. How can you know if we are inside a line or not? Cheers, Sergi

Comment on attachment 8516733 [details] Pull request #25812 Yes, I will be careful with regexp in the future ;) I changed my approach to avoid using regular expressions. This fixes it for this particular and other similar cases, but it would fail for some extreme corner cases where we have a field with the characters '="' in it. I think that to be as close to the specification as possible we have to use regexp or change lots of other things in the parser. If it is good enough for you, I will start doing some unit tests and then I will ask for the final review. If not, please tell me what you don't like. Thanks.

Comment on attachment 8516733 [details] Pull request #25812 Tests added.

Comment on attachment 8516733 [details] Pull request #25812 LGTM please rebase and land once you get a green tree thanks

Saw was already fixed in master when I tried to rebase my code. The landed fix includes tests so nothing in my PR will add anything useful.