Currently, the regex used by locale_language_from_browser() to match language strings will incorrectly match invalid language strings. As an example, the random 10-character string iecRswK4eh will incorrectly result in eh-oh-laa-laa because there is a numeral in the string, and the regex does not require that each string be preceeded by the beginning of the string or a comma (with optional whitespace).

CommentFileSizeAuthor
#8 core-failing_langage_test-1513520-8.patch735 bytespdrake
#7 core-fix_nonspecific_language_matching_regex-1513520-7.patch1.54 KBplach
#3 core-failing_langage_test-1513520-3.patch755 bytespdrake
#1 core-fix_nonspecific_language_matching_regex-1513520-1.patch1.58 KBpdrake
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pdrake’s picture

pdrake CreditAttribution: pdrake commented
Status: Active » Needs review
FileSize
core-fix_nonspecific_language_matching_regex-1513520-1.patch1.58 KB
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: +D8MI, +sprint, +language-base

Looks good. It just adds a look-behind pattern before the existing pattern. Can you also submit just the test change so we can demonstrate it failing? Thanks!

pdrake’s picture

pdrake CreditAttribution: pdrake commented
FileSize
core-failing_langage_test-1513520-3.patch755 bytes

This patch includes both an added test for a comma-space separated set of languages which should pass (and the change continues to pass) as well as the test for the bad string, which should fail.

Status: Needs review » Needs work

The last submitted patch, core-failing_langage_test-1513520-3.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs work » Reviewed & tested by the community

Looks like this is RTBC. Patch to be committed in #1 (kudos ;)

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Version: 8.x-dev » 7.x-dev
Status: Fixed » Reviewed & tested by the community
FileSize
core-fix_nonspecific_language_matching_regex-1513520-7.patch1.54 KB

Just applied the patch in #1 with -p2 and rerolled, thus setting back to RTBC.

pdrake’s picture

pdrake CreditAttribution: pdrake commented
FileSize
core-failing_langage_test-1513520-8.patch735 bytes

And here is a re-roll of #3 which should show the failure in D7.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, core-failing_langage_test-1513520-8.patch, failed testing.

pdrake’s picture

pdrake CreditAttribution: pdrake commented
Status: Needs work » Reviewed & tested by the community

Since the patch in #8 was intended to fail - setting this back to RTBC (patch in #7).

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: locale_language_from_browser() incorrectly matches invalid strings » HEAD BROKEN: locale_language_from_browser() incorrectly matches invalid strings
Priority: Normal » Critical
Status: Fixed » Reviewed & tested by the community

The test-only patch from #8 was committed. We need a rollback, and/or just to commit the fix part from #7. :)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Please commit the fix :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: HEAD BROKEN: locale_language_from_browser() incorrectly matches invalid strings » locale_language_from_browser() incorrectly matches invalid strings
Status: Reviewed & tested by the community » Fixed

Doh. :( Core commit sprees after 1 am--;

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Priority: Critical » Normal

Yay.

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: -sprint

Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
CreditAttribution: cweagans commented
Issue tags: +Needs backport to D7

Updating tags per http://drupal.org/node/1517250