Intl: CanonicalizeLanguageTag is incorrect for four-letter language codes

RESOLVED FIXED in Firefox 52

Status

()

Core
JavaScript: Internationalization API
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla52
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0
Build ID: 20130409194949

Steps to reproduce:

Four characters subtags can either be script subtags or (future reserved) language code subtags. The latter option is not handled in CanonicalizeLanguageTag. 

Also the first if-statement in the first while-loop handles a condition which can never be fulfilled. Namely singleton subtags at the start of the input string which are not privateuse subtags. Per the comment in the file, the condition was added for irregular language tags, but those are already handled in the langTagMappings structure.
Mass-moving existing Intl-related bugs to the new Core :: JavaScript: Internationalization API component.

If you think this bug has been moved in error, feel free to move it back to Core :: JavaScript Engine.

[Mass change filter: core-js-intl-api-move]
Component: JavaScript Engine → JavaScript: Internationalization API
Assignee: general → nobody
(Assignee)

Comment 2

a year ago
Created attachment 8791740 [details] [diff] [review]
four-lang.patch

Very important, many use, so exciting, much impact. :-)
Assignee: nobody → andrebargull
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8791740 - Flags: review?(jwalden+bmo)
Comment on attachment 8791740 [details] [diff] [review]
four-lang.patch

Review of attachment 8791740 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/Intl.js
@@ +393,5 @@
>          // In the example, we break at "u".
>          if (subtag.length === 1 && (i > 0 || subtag === "x"))
>              break;
>  
> +        if (i !== 0 && subtag.length === 4) {

i !== 0 applies to both if/else here, so have

  if (i !== 0) {
    if (subtag.length === 4) {
      ...
    } else if (subtag.length === 2) {
      ...
    }
  }

::: js/src/tests/Intl/four-letter-language-codes.js
@@ +2,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +// So many non-existent four letter language codes to pick from.

A++++ comment there
Attachment #8791740 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 4

a year ago
Created attachment 8792561 [details] [diff] [review]
four-lang.patch

Updated patch, carrying r+ from Waldo.
Attachment #8791740 - Attachment is obsolete: true
Attachment #8792561 - Flags: review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 5

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a5bd50d8ff9
Handle four-character language codes in CanonicalizeLanguageTag. r=Waldo
Keywords: checkin-needed

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2a5bd50d8ff9
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.