1003246 - Invalid assertion in nsDateTimeFormatMac

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Martin Thomson [:mt:]]

The MAC date formatter has an invalid assertion when forcing the use of 24H times.  This pulls the date format string from the OS date formatter and does `tr 'ha' 'HA'`.  These strings usually contain repeated instances of 'H' characters[1], but the code asserts if it finds more than one.  This line[2] should be removed or changed to:

    NS_ASSERTION(replaceCount < 0 || replaceCount > 2, "Unexpected number of \"h\" occurrences");

[1] http://www.unicode.org/reports/tr35/tr35-31/tr35-dates.html#Date_Format_Patterns
[2] http://dxr.mozilla.org/mozilla-central/source/intl/locale/src/mac/nsDateTimeFormatMac.cpp#191

Comment 1

[Martin Thomson [:mt:]]

Looks like there are cases where 'h' or 'a' don't appear at all, e.g., [1].  These places shouldn't trigger the assertion, so inverting the logic (!), I get:

    NS_ASSERTION(replaceCount <= 2, "Unexpected number of \"h\" occurrences");

[1] http://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsNSSCertValidity.cpp

Comment 2

[Martin Thomson [:mt:]]

Attachment: 0001-Bug-1003246-Loosening-assertion-to-allow-for-other-v.patch

Assertion changes only.

Comment 3

[Masatoshi Kimura [:emk]]

Comment on attachment 8414608 [details] [diff] [review]
0001-Bug-1003246-Loosening-assertion-to-allow-for-other-v.patch

jshin no longer works at Mozilla. Asking jwatt as he wrote the code.

Comment 4

[Jonathan Watt [:jwatt]]

The "Fixed Formats" section of:

https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/DataFormatting/Articles/dfDateFormatting10_4.html

says that the format is defined by the Unicode Technical Standard #35:

http://www.unicode.org/reports/tr35/tr35-31/tr35-dates.html#Date_Format_Patterns

And that says:

  Characters may be used multiple times. For example, if y is used
  for the year, 'yy' might produce '99', whereas 'yyyy' produces '1999'.

So allowing "hh" clearly seems correct.

Above the quoted text there is also an example showing "yyyyy.MMMM.dd GGG hh:mm aaa" resolving to "01996.July.10 AD 12:08 PM", which seems to make clear that multiple "a"s are allowed.

Comment 5

[Jonathan Watt [:jwatt]]

That said, it's probably a good idea to not support more than two "h"s or more than one "a".

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Can you please run this through Try if you haven't already done so? And post a link here if you did :)

Thanks!

Comment 7

[Martin Thomson [:mt:]]

https://tbpl.mozilla.org/?tree=Try&rev=bcf3dc54722d

Comment 8

[Martin Thomson [:mt:]]

It's not done yet, but I'm prepared to call that a success.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/608d04865f00

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/608d04865f00