From 7d24542a7d7ea1a1da14a3b1875b51ae1a6f4d6f Mon Sep 17 00:00:00 2001
From: Moritz Bunkus <moritz@bunkus.org>
Date: Mon, 7 Sep 2020 20:23:04 +0200
Subject: [PATCH] BCP 47: format includes all components even if language is
 empty

Before there was a shortcut: if language is empty, only private use
was formatted as that's the only possible way to produce a valid
tag. However, the GUI uses formatting of potentially invalid tags and
then parsing them again as a test for validity. Therefore the
formatting function must always format all individual components.

Fixes #2924.
---
 NEWS.md                     |  4 ++++
 src/common/bcp47.cpp        | 15 +++++----------
 tests/unit/common/bcp47.cpp | 10 ++++++++++
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index a28a9913a..200941355 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -12,6 +12,10 @@
 * MKVToolNix GUI: IETF BCP 47 language tags: the "variants" combo-boxes were
   not populated even when the language tag was valid and contained at a
   variant. Fixes #2923.
+* MKVToolNix GUI: IETF BCP 47 language tags: when no language is selected, at
+  least one of the other components (extended subtags, region, or variants)
+  has something selected and "private use" is not empty, the GUI would claim
+  this to be a valid tag, which it isn't. Fixes #2924.
 
 
 # Version 50.0.0 "Awakenings" 2020-09-06
diff --git a/src/common/bcp47.cpp b/src/common/bcp47.cpp
index ee94715cc..ae667650c 100644
--- a/src/common/bcp47.cpp
+++ b/src/common/bcp47.cpp
@@ -79,15 +79,6 @@ language_c::format_internal(bool force)
   if (!m_valid && !force)
     return {};
 
-  if (m_language.empty()) {
-    auto output = "x"s;
-
-    for (auto const &private_use : m_private_use)
-      output += fmt::format("-{}", mtx::string::to_lower_ascii(private_use));
-
-    return output;
-  }
-
   auto output = mtx::string::to_lower_ascii(m_language);
 
   for (auto const &subtag : m_extended_language_subtags)
@@ -106,7 +97,11 @@ language_c::format_internal(bool force)
     output += fmt::format("-{}", mtx::string::to_lower_ascii(extension));
 
   if (!m_private_use.empty()) {
-    output += "-x";
+    if (!output.empty())
+      output += "-";
+
+    output += "x";
+
     for (auto const &private_use : m_private_use)
       output += fmt::format("-{}", mtx::string::to_lower_ascii(private_use));
   }
diff --git a/tests/unit/common/bcp47.cpp b/tests/unit/common/bcp47.cpp
index bb341fbe6..d213f1408 100644
--- a/tests/unit/common/bcp47.cpp
+++ b/tests/unit/common/bcp47.cpp
@@ -79,6 +79,16 @@ TEST(BCP47LanguageTags, Formatting) {
   EXPECT_EQ("x-weee-wooo"s, l.format_long(true));
 }
 
+TEST(BCP47LanguageTags, FormattingInvalidWithoutLanguage) {
+  auto l = mtx::bcp47::language_c{};
+
+  l.set_region("FR"s);
+  l.set_private_use({ "moo"s });
+
+  EXPECT_EQ(""s,          l.format());
+  EXPECT_EQ("-FR-x-moo"s, l.format(true));
+}
+
 TEST(BCP47LanguageTags, CodeConversion) {
   EXPECT_EQ(""s,    mtx::bcp47::language_c{}.get_iso639_2_code());
   EXPECT_EQ("ger"s, mtx::bcp47::language_c::parse("de").get_iso639_2_code());