codebude / QRCoder

A pure C# Open Source QR Code implementation
MIT License
4.6k stars 1.09k forks source link

ContactData QR Code generation is wrong #581

Open EscolarProgramming opened 6 days ago

EscolarProgramming commented 6 days ago

Type of issue

[x] Bug
[ ] Question (e.g. about handling/usage)
[ ] Request for new feature/improvement

Expected Behavior

Locality, Postal Code and Region should be in the correct fields

Current Behavior

The fields mentioned are in the wrong order. On Android, they are usually read anyway and the values are then entered in the wrong fields. On iOS, fields with incorrect values are simply omitted. As a result, the city name, for example, is never transferred.

Possible Solution (optional)

https://github.com/codebude/QRCoder/blob/master/QRCoder/PayloadGenerator/ContactData.cs#L114 The creation of the string is wrong. Currently: {street} {houseNumber}, {zipCode}, {city}, {stateRegion}, {country} Correct: {street} {houseNumber}, {city}, {stateRegion}, {zipCode}, {country}

Edit: Here's the corresponding RFC: https://www.rfc-editor.org/rfc/rfc6350.html#section-6.3.1 Section 6.3.1.: ADR-value = ADR-component-pobox ";" ADR-component-ext ";" ADR-component-street ";" ADR-component-locality ";" ADR-component-region ";" ADR-component-code ";" ADR-component-country

digitaldirk commented 5 days ago

Would you like to do a PR? If not I could do one. @EscolarProgramming

EscolarProgramming commented 5 days ago

@digitaldirk I can only create a PR at the beginning of next week, if you can do it earlier, feel free to do it yourself

codebude commented 5 days ago

Hi @EscolarProgramming ,

thanks for your feedback. Two notes:

https://github.com/codebude/QRCoder/blob/master/QRCoder/PayloadGenerator/ContactData.cs#L114 The creation of the string is wrong.

You're pointing to line 114 which is part of the MeCard address handling. Do you really mean the MeCard behaviour or did you mean vCard (lines 193ff -> https://github.com/codebude/QRCoder/blob/master/QRCoder/PayloadGenerator/ContactData.cs#L193 )

To get it right. You expect the behaviour of AddressOrder.Reversed mode, but with street + housenumber in European format (as in AddressOrder.Default) correct?

digitaldirk commented 5 days ago

@codebude I'll leave this one to you (after looking into I got a little confused and feel like you're more qualified with the US vs Europe formatting 😃)

I did however convert that file to use stringbuilder instead of string concatenation if you want a seperate PR (and maybe I could go through and find more places to do this as well?)

Thanks

EscolarProgramming commented 4 days ago

@codebude You are right, I meant vCard. Bug reporting on my mobile phone ist hard :D I don‘t know if the MeCard one is wrong too.

And yes, your second paragraph is my expected behaviour (and in my opinion the right one in regards of the RFC).