76714 2012-01-20 10:21:13 -0800 implement Date.toLocaleString() using ICU 2012-03-05 07:55:29 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 efidler efidler ap darin webkit.review.bot oldest_to_newest 539603 0 efidler 2012-01-20 10:21:13 -0800 Some platforms don't localize strftime well, so use ICU to implement Date.toLocaleString() 539607 1 123341 efidler 2012-01-20 10:29:09 -0800 Created attachment 123341 Patch 539649 2 123341 darin 2012-01-20 11:29:13 -0800 Comment on attachment 123341 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123341&action=review > Source/JavaScriptCore/runtime/DatePrototype.cpp:210 > + UDateFormat* df = udat_open(timeStyle, dateStyle, 0, 0, -1, 0, 0, &status); Since the point of these functions is to format for the current locale, it seems critical to pass in a suitable locale string. See, for example, what is done for text break locales in the currentTextBreakLocaleID function. I’m surprised that is OK to pass 0 for the locale string. 539764 3 efidler 2012-01-20 13:46:04 -0800 0 is the "default locale". We rely on ICU for localization in many areas, so apps will have the default locale set appropriately to the desired locale. I guess that's not true for OSX or IOS, but there's a different path here for that. I could put the strftime path into HAVE(STRFTIME) or make a USE(ICU_DEFAULT_LOCALE) if you prefer. 540207 4 darin 2012-01-21 19:02:02 -0800 (In reply to comment #3) > 0 is the "default locale" Is that a general ICU feature, or one specific to RIM platforms? I ask because I don’t see it mentioned in ICU documentation. > We rely on ICU for localization in many areas Sure, OS X and iOS do too. My question is about how the ICU API works. > I could put the strftime path into HAVE(STRFTIME) or make a USE(ICU_DEFAULT_LOCALE) if you prefer. Since this code passes 0 for the locale it should be used only on platforms where that will do the right thing. To get the #if right we need to know how where that “0 means default locale” feature exists. 540330 5 ap 2012-01-22 16:51:10 -0800 This may not be a regression from strftime, but the behavior would not be correct when locale changes at runtime, especially with WebKit2. In Mac Safari, output of (new Date).toLocaleString() changes immediately after a user changes the format in system preferences. 540616 6 efidler 2012-01-23 09:45:21 -0800 NULL for default locale is an ICU-standard feature: "Using the ICU C functions, NULL can be passed for a locale parameter to specify the default locale." from: http://userguide.icu-project.org/locale#TOC-Default-Locales For Alexey's point, Date.toLocaleString() will always use the current default locale. If that is changed using uloc_setDefault(), the new value should be immediately take effect. 542280 7 123341 webkit.review.bot 2012-01-25 08:09:01 -0800 Comment on attachment 123341 Patch Rejecting attachment 123341 from commit-queue. New failing tests: media/audio-garbage-collect.html Full output: http://queues.webkit.org/results/11342538 542720 8 efidler 2012-01-25 15:54:07 -0800 flaky test looks unrelated, so let's try cq+ again 542741 9 123341 webkit.review.bot 2012-01-25 16:11:50 -0800 Comment on attachment 123341 Patch Clearing flags on attachment: 123341 Committed r105939: <http://trac.webkit.org/changeset/105939> 542742 10 webkit.review.bot 2012-01-25 16:11:54 -0800 All reviewed patches have been landed. Closing bug. 542762 11 mrowe 2012-01-25 16:41:40 -0800 This broke all of the Mac builds on build.webkit.org as unicode/udat.h is not available. 542768 12 mrowe 2012-01-25 16:48:19 -0800 I checked in a fix in r105943 to try and fix the Mac builds. 570695 13 ap 2012-03-05 07:55:29 -0800 This has reportedly caused a regression, bug 80262. 123341 2012-01-20 10:29:09 -0800 2012-01-25 16:11:50 -0800 Patch bug-76714-20120120132907.patch text/plain 2446 efidler U3VidmVyc2lvbiBSZXZpc2lvbjogMTA1NDEwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA0 NmMwM2RjN2M4NWYzY2M4ZWY1MGI0NDA3Y2EwYTk5NTI5Zjc0ZWM2Li5mNDY4MTViMzlmYWE2ZGVk NmUwMGNhNWUzYTBiOWNiZTBiNzU4OGU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMyBAQAorMjAxMi0wMS0yMCAgRWxpIEZpZGxlciAgPGVmaWRsZXJAcmltLmNvbT4KKwor ICAgICAgICBJbXBsZW1lbnQgRGF0ZS50b0xvY2FsZVN0cmluZygpIHVzaW5nIElDVQorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzY3MTQKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHJ1bnRpbWUvRGF0ZVBy b3RvdHlwZS5jcHA6CisgICAgICAgIChKU0M6OmZvcm1hdExvY2FsZURhdGUpOgorCiAyMDEyLTAx LTE4ICBSb2xhbmQgVGFrYWNzICA8dGFrYWNzLnJvbGFuZEBzdHVkLnUtc3plZ2VkLmh1PgogCiAg ICAgICAgIENyb3NzLXBsYXRmb3JtIHByb2Nlc3NvciBjb3JlIGNvdW50ZXIgZml4CmRpZmYgLS1n aXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9EYXRlUHJvdG90eXBlLmNwcCBiL1Nv dXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0RhdGVQcm90b3R5cGUuY3BwCmluZGV4IDk2ZGVh MDFkYmUzYzM2ZGY1MjBhZjdiZmFlMzAwMTg4ZGQ5YWMzZGEuLjQ5ZjU2YmM3NjdhODk0NDg3NDFm Nzc4MGVlYmZlYmMzMTA5ZjlkNzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9y dW50aW1lL0RhdGVQcm90b3R5cGUuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50 aW1lL0RhdGVQcm90b3R5cGUuY3BwCkBAIC02NCw2ICs2NCwxMCBAQAogI2luY2x1ZGUgPENvcmVG b3VuZGF0aW9uL0NvcmVGb3VuZGF0aW9uLmg+CiAjZW5kaWYKIAorI2lmIFVTRShJQ1VfVU5JQ09E RSkKKyNpbmNsdWRlIDx1bmljb2RlL3VkYXQuaD4KKyNlbmRpZgorCiAjaWYgT1MoV0lOQ0UpICYm ICFQTEFURk9STShRVCkKIGV4dGVybiAiQyIgc2l6ZV90IHN0cmZ0aW1lKGNoYXIgKiBjb25zdCBz LCBjb25zdCBzaXplX3QgbWF4c2l6ZSwgY29uc3QgY2hhciAqIGNvbnN0IGZvcm1hdCwgY29uc3Qg c3RydWN0IHRtICogY29uc3QgdCk7IC8vcHJvdmlkZWQgYnkgbGliY2UKICNlbmRpZgpAQCAtMTk1 LDcgKzE5OSwyOSBAQCBzdGF0aWMgSlNDZWxsKiBmb3JtYXRMb2NhbGVEYXRlKEV4ZWNTdGF0ZSog ZXhlYywgRGF0ZUluc3RhbmNlKiwgZG91YmxlIHRpbWVJbk1pbAogICAgIHJldHVybiBqc05vbnRy aXZpYWxTdHJpbmcoZXhlYywgVVN0cmluZyhidWZmZXIsIGxlbmd0aCkpOwogfQogCi0jZWxzZSAv LyAhUExBVEZPUk0oTUFDKSAmJiAhUExBVEZPUk0oSU9TKQorI2VsaWYgVVNFKElDVV9VTklDT0RF KSAmJiAhVUNPTkZJR19OT19GT1JNQVRUSU5HCisKK3N0YXRpYyBKU0NlbGwqIGZvcm1hdExvY2Fs ZURhdGUoRXhlY1N0YXRlKiBleGVjLCBEYXRlSW5zdGFuY2UqIGRhdGVPYmplY3QsIGRvdWJsZSB0 aW1lSW5NaWxsaXNlY29uZHMsIExvY2FsZURhdGVUaW1lRm9ybWF0IGZvcm1hdCkKK3sKKyAgICBV RGF0ZUZvcm1hdFN0eWxlIHRpbWVTdHlsZSA9IChmb3JtYXQgIT0gTG9jYWxlRGF0ZSA/IFVEQVRf TE9ORyA6IFVEQVRfTk9ORSk7CisgICAgVURhdGVGb3JtYXRTdHlsZSBkYXRlU3R5bGUgPSAoZm9y bWF0ICE9IExvY2FsZVRpbWUgPyBVREFUX0xPTkcgOiBVREFUX05PTkUpOworCisgICAgVUVycm9y Q29kZSBzdGF0dXMgPSBVX1pFUk9fRVJST1I7CisgICAgVURhdGVGb3JtYXQqIGRmID0gdWRhdF9v cGVuKHRpbWVTdHlsZSwgZGF0ZVN0eWxlLCAwLCAwLCAtMSwgMCwgMCwgJnN0YXR1cyk7CisgICAg aWYgKCFkZikKKyAgICAgICAgcmV0dXJuIGpzRW1wdHlTdHJpbmcoZXhlYyk7CisKKyAgICBVQ2hh ciBidWZmZXJbMTI4XTsKKyAgICBpbnQzMl90IGxlbmd0aDsKKyAgICBsZW5ndGggPSB1ZGF0X2Zv cm1hdChkZiwgdGltZUluTWlsbGlzZWNvbmRzLCBidWZmZXIsIDEyOCwgMCwgJnN0YXR1cyk7Cisg ICAgdWRhdF9jbG9zZShkZik7CisgICAgaWYgKHN0YXR1cyAhPSBVX1pFUk9fRVJST1IpCisgICAg ICAgIHJldHVybiBqc0VtcHR5U3RyaW5nKGV4ZWMpOworCisgICAgcmV0dXJuIGpzTm9udHJpdmlh bFN0cmluZyhleGVjLCBVU3RyaW5nKGJ1ZmZlciwgbGVuZ3RoKSk7Cit9CisKKyNlbHNlCiAKIHN0 YXRpYyBKU0NlbGwqIGZvcm1hdExvY2FsZURhdGUoRXhlY1N0YXRlKiBleGVjLCBjb25zdCBHcmVn b3JpYW5EYXRlVGltZSYgZ2R0LCBMb2NhbGVEYXRlVGltZUZvcm1hdCBmb3JtYXQpCiB7Cg==