jroal / a2dpvolume

Automatically exported from code.google.com/p/a2dpvolume
http://jimroal.com/slist.htm
95 stars 33 forks source link

Formatting of Link to Google Maps uses US localization for Date/Time and does not encode URL components #57

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
During converting the string.xml for the German translation, I noticed the 
following line in the code of StoreLoc.java:

Time t = new Time();
t.set((long) l4.getTime());
String temp = "http://maps.google.com/maps?q="
    + l4.getLatitude() + "," + l4.getLongitude() + "+"
    + "(" + car + " " + t.format("%D, %r") + " acc="
    + df.format(l4.getAccuracy()) + ")";

This code has problems with Non-US locations and misses to escape special 
bluetooth device names:

* t.format("%D, %r") always formats in 12 hour clock with "am/pm", which is 
only used in US/UK/AUS but nowhere else on the world. The formatting of the 
date should respect the locale settings of the operating system, which is not 
used here (it only "translates" a.m./p.m. to a regional string, but this is 
also bogus in countries with 24h clock). The code should use the class 
android.text.format.DateUtils, which uses phone's locale settings (e.g. 
DateUtils.formatDateTime(context, l4.getTime(), DateUtils.FORMAT_ABBREV_ALL) - 
or similar flags value, without explicitely stating 12/24h clock. See Android 
documentation)

* Also the problem with this code is missing URL escaping (needs to be 
UTF8-escaped using %XX). My bluetooth name is "Blue & Me" which corrupts the 
string completely because "&" is not escaped. The Java Class to correctly 
escape URLs is: java.net.URLEncoder. Code that correctly encodes the contents 
of the q= parameter would be:

String locStr = l4.getLatitude() + "," + l4.getLongitude() + " " + "(" + car + 
" " + DateUtils.formatDateTime(context, l4.getTime(), 
DateUtils.FORMAT_ABBREV_ALL) + " acc=" + df.format(l4.getAccuracy()) + ")";
String temp = "http://maps.google.com/maps?q=" + URLEncoder.encode(locStr, 
"UTF-8");

Please note "UTF-8", as Google maps expects UTF-8 encoding of URL parameters.

I will now proceed with the translations.

Original issue reported on code.google.com by uwe.h.schindler on 7 Aug 2011 at 6:20

GoogleCodeExporter commented 9 years ago

Original comment by JimR...@gmail.com on 7 Aug 2011 at 6:32

GoogleCodeExporter commented 9 years ago
Fixed in 2.0.14 now available for download.  Thanks for all the detailed help!  
That made this easy.  Please test 2.0.14 and confirm it is fixed.

Original comment by JimR...@gmail.com on 7 Aug 2011 at 8:41

GoogleCodeExporter commented 9 years ago
Hi,

the Date/Time is now displayed in my own local time format (German), so all is 
fine with this. The time is displayed in 24 hour notation here, too.

The whole google maps URL creation is still a little bit fishy. You should 
encode the *whole* query string, not only the car name by URLEncoder (including 
the date, time, "(", ")" and "acc=" parts). URLEncoder will automatically 
encode spaces to "+" and all special characters that could appear anywhere in 
the whole query string. The exception can never-ever be thrown (as UTF-8 is a 
"required" encoding for all Java machines).

First build the full description including lat/lon and the parenthesis and 
after that encode it. All this is passed as REST param "q" to the Gmaps URL. 
You should of course not encode the full URL, only the part following the "q=".

See my "simplified" code above, please note that I removed your "manual" 
encoding of the space to "+" after the latlon pair and replace it by a simple 
space.

Original comment by uwe.h.schindler on 7 Aug 2011 at 9:14

GoogleCodeExporter commented 9 years ago
I looked into encoding the whole thing but realized that only the car really 
needed it.  The rest is not localized and does not need to be, and it already 
works in the URL fine.  The encoding to UTF-8 can throw an exception so I 
needed to deal with that in a try-catch.  

Replacing a space with _ is needed to properly store the file.  It would not 
work correctly as a file name to have the URL encoding.  

What is "fishy" about the URL?  Or, is your concern just about the 
implementation?

Original comment by JimR...@gmail.com on 7 Aug 2011 at 9:20

GoogleCodeExporter commented 9 years ago
> The encoding to UTF-8 can throw an exception so I needed to deal with that in 
a try-catch.

Yes that unfortunately a checked exception. I just saqy, UTF-8 is required, so 
it can never happen. In general this handled by "rethrowing" as 
RuntimeException or Error (as it is impossible).

> The rest is not localized and does not need to be, and it already works in 
the URL fine.  The encoding to UTF-8 can throw an exception so I needed to deal 
with that in a try-catch. 

E.g. the date/time could contain some special character thats not allowed in 
URL (maybe in some chinese locales or whatever). So in general when 
constructing URLs the parts before and after the "=" must be encoded ("q" is 
always safe, so useless to encode). The part of the URL that comes before the ? 
is also fixed and given by Google, so no encoding needed (it would be wrong to 
do). The only remaining part is the query string. To be safe, produce it as 
plain text like this:

final String locStr = l4.getLatitude() + "," + l4.getLongitude() + " (" + car + 
" " + DateUtils.formatDateTime(context, l4.getTime(), 
DateUtils.FORMAT_ABBREV_ALL) + " acc=" + df.format(l4.getAccuracy()) + ")";

(please note that the "+(" was replaced by " (")

After that encode this plain text string using URLEncode and append it to the 
final Google URL:

final String locStrEnc;
try {
  locStrEnc = URLEncoder.encode(locStr, "UTF-8");
} catch (UnsupportedEncodingException uee) {
  // can never happen, as UTF-8 is always available (its required)
  throw new RuntimeException("Something gone totally wrong", uee);
}
final String temp = "http://maps.google.com/maps?q=" + locStrEnc;

I am not affected by this, this is just a recommendation to prevent such bugs 
and enforce "correct style".

Original comment by uwe.h.schindler on 7 Aug 2011 at 9:33

GoogleCodeExporter commented 9 years ago
OK, so I tried to encode the whole URL string and now it does not work.  Google 
Maps expects lat, long and then +(item info).  The encoder codes the + into 
some hex and the URL no longer works.

Original comment by JimR...@gmail.com on 7 Aug 2011 at 10:30

GoogleCodeExporter commented 9 years ago
> Google Maps expects lat, long and then +(item info).

This is why I said, use: "lat,long (item info)" (no plus). Spaces are converted 
to "+" by URLEncoder, which is what happens during URL encoding. This is the 
reason why Google expects a "+" sign. The documentation on the wiki page is a 
little bit misleading.

Original comment by uwe.h.schindler on 7 Aug 2011 at 10:35

GoogleCodeExporter commented 9 years ago
OK, I got it all working right now and I am url encoding the URL as you 
advised.  Thanks for all the help.  Fixed in 2.0.15 available for download in a 
few seconds.

Original comment by JimR...@gmail.com on 7 Aug 2011 at 11:06

GoogleCodeExporter commented 9 years ago
Works fine! The shorter date/time format is better, too (I have seen it in the 
HG commit and tried it out).

Small comment on translation follows in eMail.

Original comment by uwe.h.schindler on 7 Aug 2011 at 11:23