Yesterday Red Hat upgraded its internal CI to Fedora 32 (ICU 65.1), causing stress/intl-datetimeformat-formatrange-relevant-extensions.js and stress/intl-datetimeformat-formatrange.js to begin failing. The problem is that the space characters used in the range format changed at some point. The current version of the test expects normal ASCII space characters for ICU 67 and newer, and special spaces for older versions of ICU. The test passes for me locally on Fedora 33 (ICU 67.1), so perhaps we just need to use the new codepath for ICU 65 and 66 as well. I'm going to try this first, but I'm not sure if it will pass EWS. Problem is, we seem to have strange differences in behavior between Apple and Linux. So if this fails EWS, we'll need to find some more creative way to ensure the test passes regardless of which space character ICU decides to use.
Created attachment 413718 [details] Patch
Comment on attachment 413718 [details] Patch r=me
<rdar://problem/71257288>
Committed r269667: <https://trac.webkit.org/changeset/269667> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413718 [details].
Reopening since these tests are broken again after bug #213822. I'll look closer and try to come up with something next week.
(In reply to Michael Catanzaro from comment #5) > next week. Ah well, better late than never!
Created attachment 415561 [details] Patch
Comment on attachment 415561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415561&action=review > JSTests/stress/intl-datetimeformat-formatrange-relevant-extensions.js:51 > - shouldBe(fmt1.formatRange(date1, date2), `ä¸/ä¸ã/ãä¸, äº:ãã${range}ä¸:ãã AM`); > - shouldBe(fmt1.formatRange(date1, date3), `ä¸/ä¸ã/ãä¸, äº:ãã AM${range}ä¸/äºã/ãä¸, äº:ãã AM`); > + shouldBe(fmt1.formatRange(date1, date2), `ä¸/ä¸ã/ãä¸, äº:ãã â ä¸:ãã AM`); > + shouldBe(fmt1.formatRange(date1, date3), `ä¸/ä¸ã/ãä¸, äº:ãã AM â ä¸/äºã/ãä¸, äº:ãã AM`); Of course these diffs are unreadable in the Bugzilla review tool, since it doesn't use UTF-8....
Comment on attachment 415561 [details] Patch jsc EWS is unhappy. I'll need to look over its results.
Created attachment 415592 [details] Patch
Comment on attachment 415592 [details] Patch r=me
Comment on attachment 415592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415592&action=review > JSTests/stress/intl-datetimeformat-formatrange-relevant-extensions-ja.js:5 > + if (Object.prototype.toString.call(actual) === '[object String]') We can just do, `typeof actual === 'string'`.
Thanks for reviewing. (In reply to Yusuke Suzuki from comment #12) > We can just do, `typeof actual === 'string'`. I think it will work in this case since everything is a string literal or template, but StackOverflow says to avoid it since it has a tendency to return garbage results: see https://stackoverflow.com/a/7894320/1120203 linking to http://bonsaiden.github.io/JavaScript-Garden/#types. Do you want me to try changing this anyway?
(In reply to Michael Catanzaro from comment #13) > Thanks for reviewing. > > (In reply to Yusuke Suzuki from comment #12) > > We can just do, `typeof actual === 'string'`. > > I think it will work in this case since everything is a string literal or > template, but StackOverflow says to avoid it since it has a tendency to > return garbage results: see https://stackoverflow.com/a/7894320/1120203 > linking to http://bonsaiden.github.io/JavaScript-Garden/#types. > > Do you want me to try changing this anyway? Yes, I think `typeof` is better. `Object.prototype.toString.call(actual)` returns `"[object String]"` when, 1. The input is actually String. Then "ToObject" abstract operation will be performed and it is converted to String object. Then, `Object.prototype.toString` returns "[object String]" 2. The input is weird String object (new String("Hello")). Then it just returns "[object String]" On the other hand, `typeof actual` returns "string" only when `actual` is actually string primitive type. It returns "object" when the input is String object. In this case, we will never pass String object. If String object appears, rather, this is a bug. The stack overflow entry attempts to find what class the given object is. `typeof` is not appropriate for that purpose. But here, what we would like to know is whether the input is string primitive type (not object), not a class of object. So `typeof` is the best.
OK, 'typeof' it is. I don't pretend to understand JavaScript, anyway. :D
Created attachment 415798 [details] Patch
Committed r270608: <https://trac.webkit.org/changeset/270608> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415798 [details].