This is a followup to bug 128677 comment 24. Jeff wrote: " Oh, ugh. I just realized, this quite possibly/probably will result (since we're interpeting these strings as null-terminated) that this probably messes up handling of "dates/fields/year\0EXTRA". Sigh. Run with this for now, but file a followup (assign to me) to figure out how to address this. I have an idea for fixing this, but it's probably not worth me talking you through executing it. "

A couple mini-cleanupish patches before the ultimate fix.

This makes ComputeDisplayNames a bit more readable overall. This could be a lambda, now -- but if I did that, the next patch (which templatizes this stuff based on string character type) would be a no-go without C++14's template lambdas that we can't quite use yet. I could say it later, or now, but there's an argument to have about whether templating this parsing, or just copying to a two-byte chars array and parsing only that version, is preferable. Efficiency doesn't matter much. Mostly I do this because forked handling is generally pretty common and it seems best not to deviate. I suspect un-templating and dealing only in two-byte things could be done if desired -- but probably best to say so wrt *this* patch, because moving this code as few times as desirable seems preferable.

Missing one mini-fix.

Working with two different iterators is distinctly less pleasant than working with one plus null-termination. Not sure anything can be done about it -- prescanning for nulls seems not worth it enough to do, right?

Comment on attachment 8816353 [details] [diff] [review] Rearrange a few lines Review of attachment 8816353 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/Intl.cpp @@ +3047,2 @@ > // 1. Assert: locale is a string. > MOZ_ASSERT(args[0].isString()); this assertion won't be necessary if we're doing toString in next line. @@ +3054,2 @@ > // 2. Assert: style is a string. > MOZ_ASSERT(args[1].isString()); here too. @@ +3061,2 @@ > // 3. Assert: keys is an Array. > MOZ_ASSERT(args[2].isObject()); and here.

Comment on attachment 8816362 [details] [diff] [review] Parametrize ComputeSingleDisplayName based on the character type of the key string, and iterate through the string using iterators, not using null-termination Review of attachment 8816362 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/Intl.cpp @@ +3330,5 @@ > for (uint32_t i = 0; i < keys->length(); i++) { > if (!GetElement(cx, keys, keys, i, &v)) > return false; > > pattern.clear(); this and the definition of pattern should be removed now.

Pushed by jwalden@mit.edu: https://hg.mozilla.org/integration/mozilla-inbound/rev/cbaaf07a1191 Add a slash-matching function rather than using MatchPart. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/d99c50a89c38 Rearrange a few lines. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/97aef98c5230 Move display-name computation into its own function. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/e70365b0720c Parametrize ComputeSingleDisplayName based on the character type of the key string, and iterate through the string using iterators, not using null-termination. r=arai

https://hg.mozilla.org/mozilla-central/rev/cbaaf07a1191 https://hg.mozilla.org/mozilla-central/rev/d99c50a89c38 https://hg.mozilla.org/mozilla-central/rev/97aef98c5230 https://hg.mozilla.org/mozilla-central/rev/e70365b0720c