Needs review
Project:
Ubercart
Version:
8.x-4.x-dev
Component:
Code
Priority:
Minor
Category:
Feature request
Assigned:
Unassigned
Reporter:
Anonymous (not verified)
Created:
11 Mar 2014 at 21:51 UTC
Updated:
28 Feb 2019 at 06:37 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
Anonymous (not verified) commentedComment #2
tr commentedI'm surprised no one from Canada has mentioned this before - it's always been like that in Ubercart, since the first release 7 years ago.
Minor because this is just the default, you can customize your address format at admin/store/settings/countries/formats
Apparently this is what all Canadians have been doing?
According to your link, the comma should be removed after the city, the zone_code should be used, not the zone_name, and the country should not be specified for addresses within Canada.
I think we can make this change without updating the version - forcing an update will just override any custom format that a site may have set. Patch attached.
Comment #3
tr commentedChanging version for the testbot.
Comment #4
longwaveI agree with not changing the country version at this stage as this is only a format change.
My only question is does the double space get output correctly? HTML output will generally collapse multiple spaces into one, so does this actually work as expected in all the places we output addresses? And if not, how would we handle that?
Comment #5
Anonymous (not verified) commented to enforce the second space? That's how I've always done it in my HTML.
If the Ubercart addressing format is meant to follow that country's official postal conventions as closely as possible, then the patch is fine. The most important thing is to get the city, province and postal code ordering correct (and, ideally, the double space between the province and postal code). Canadians are pretty firm about that.However, you'll find a lot of variation in punctuation and abbreviations.
A few observations about Canadian addressing conventions (or: where and why Canadians deviate from the official postal guide):
@TR: Being the chill Canadians that we are, eh, I guess the incorrect address format never really bothered us. :-P
Comment #6
Anonymous (not verified) commentedUgh. I can spell… sometimes
Comment #7
longwave might be problematic in some cases, such as where the address is included in a plaintext email. Would need some testing, I guess; maybe we should just commit this patch as-is (please RTBC TR's patch if it works for you!) and follow that up later.
Comment #8
Anonymous (not verified) commented#WorksForMe
Comment #9
longwaveCommitted #2. Needs porting to 8.x-4.x, and perhaps we should do something about the double space issue, if that is actually a problem.
Comment #10
Anonymous (not verified) commentedFixing the double space issue will make the patch pretty much 100% correct. Granted, Canadians will live fine without it. Who knows, maybe Canada Post will change the standard to a single space before you figure out how to do it. Still, if it can be done, it should be done for correctness.
Is it possible to insert a non-breaking space character through a Unicode escape or possibly some other PHP character escape.
Comment #11
longwaveCherry picked to 8.x-4.x. Back to active to figure out the double space issue at some future time.
Comment #13
Anonymous (not verified) commentedGood man, longwave. Thank you for your hard work :-)
Comment #14
tr commentedRenaming this issue and changing it to a feature request.
What we have left to do here is to figure out how to support addresses being displayed in an HTML document (on a website or in an HTML e-mail) and also support addresses in plain text (in a plain text e-mail or for export to a label printer, fulfillment house, or the like).
Right now, Address objects have a __toString() function which returns text including
<br>tags. So because we're already returning HTML here we might as well go ahead and also replace the spaces with . That will solve the HTML formatting issues.But we also need another function to return plain text. Semantically, __toString() probably should be returning the plain text version, not the HTML version like it does now. Then we can add a toHtmlString() function for when we need HTML.
Then we have to look through the Ubercart code and use the correct functions in the correct places because right now we use PHP's implicit type conversion to output addresses, which calls __toString() when we implicitly cast an Address object to a string. All those places where we actually need HTML should have an explicit toHtmlString() call instead. But since this is an implicit cast, there's no easy way to grep for this usage.
Anyway, here's a patch for the
issue, which is a first step.Comment #16
tr commentedI forgot to change the test - because we're now using
instead of spaces, we have to alter the test to expect that when we test addresses.Here's a new patch with an updated test.
Comment #18
tr commentedWhoops, missed a spot.
Comment #20
tr commentedSigh. Missed another spot.
Comment #22
tr commentedRe-rolled #20 against HEAD.