Use the (first segment of the) original locale value for the workaround again

cf7c9599e776eba8e14614cecb528d3da5778190 "Make comphelper/configuration.hxx work
for localized properties", which had originally introduced this code, had been
careful to ensure that the

>             assert(
>                 !locale.isEmpty() && locale.indexOf('-') == -1 &&
>                 locale.indexOf('_') == -1);

would always hold here (it had already removed all trailing "-..." and "_..."
segments, but had made sure to stop before `locale`, which is known to initially
be no non-empty, would have become empty, by treating a leading "-" or "_" not
as a segment delimiter, but rather as a first segment).

dfc28be2487c13be36a90efd778b8d8f179c589d "configmgr: Use a proper
LanguageTag-based locale fallback mechanism" had changed that, instead setting
`locale` to some value obtained from `LanguageTag::getFallbackStrings`, which
might or might not satisfy the assert.

But there was no good reason for that part of
dfc28be2487c13be36a90efd778b8d8f179c589d in the first place:  The workaround (as
explained in the leading code comment) was meant to be carried out with the
first segment of the original `locale` value, not with some fallback value.  So
put back here the computation of that first segment of the original `locale`
value.  (And drop the misleading empty line that
dfc28be2487c13be36a90efd778b8d8f179c589d had, for no good reason, introduced
between the workaround's leading code comment and its actual code.)

However, it turns out that there was one flaw in
cf7c9599e776eba8e14614cecb528d3da5778190:  When the original `locale` starts
with "-" or "_", the resulting `locale` representing the first segment will be
"-" or "_", so the `locale.indexOf('-') == -1 && locale.indexOf('_') == -1` part
of the assert would be false.  But that wouldn't be an issue for the following
code (the only issue would be if `locale` had become empty, in which case the
`name2.startsWiht(locale)` check would trivially become true, and that for loop
would erroneously pick the child with the empty `name2`), and that part of the
assert had merely been there to reinforce that `locale` had indeed been stripped
down to the first segment.  A correct version of the assert would have used
`locale.indexOf('-', 1) == -1 && locale.indexOf('_', 1) == -1` instead, but as
the code now makes it obvious anyway that `locale` has been cut down here to the
first segment, we can just as well simplify the assert to just the
`!locale.isEmpty()` part.

(The added test code unfortunately doesn't actually test this piece of code, and
somewhat unexpectedly receives the "default" value from the empty string locale
default, rather than the "en-US" value from the higher precedence "en-US" locale
default, because `aFallbacks` happens to contain an empty string, so we already
leave Access::getChild early in the "Find the best match using the LanguageTag
fallback mechanism, excluding the original tag" block.)

Change-Id: Ib92e714c9db4879be058529ec905e631df975424
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147113
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

2 files changed, 13 insertions, 6 deletions

diff --git a/configmgr/qa/unit/test.cxx b/configmgr/qa/unit/test.cxx
index 3de93a6672df..7aa2daf6f96d 100644
--- a/configmgr/qa/unit/test.cxx
+++ b/configmgr/qa/unit/test.cxx
@@ -324,6 +324,13 @@ void Test::testLocalizedProperty() {
             access->getByHierarchicalName("/org.libreoffice.unittest/localized/*pt") >>= v);
         CPPUNIT_ASSERT_EQUAL(OUString("pt-PT"), v);
     }
+    {
+        // Make sure a degenerate passed-in "-" locale is handled gracefully:
+        OUString v;
+        CPPUNIT_ASSERT(
+            access->getByHierarchicalName("/org.libreoffice.unittest/localized/*-") >>= v);
+        CPPUNIT_ASSERT_EQUAL(OUString("default"), v);
+    }
 }
 
 void Test::testReadCommands()
diff --git a/configmgr/source/access.cxx b/configmgr/source/access.cxx
index 48d9b46ddc26..9d71c7fa2978 100644
--- a/configmgr/source/access.cxx
+++ b/configmgr/source/access.cxx
@@ -68,6 +68,7 @@
 #include <com/sun/star/util/ElementChange.hpp>
 #include <comphelper/sequence.hxx>
 #include <comphelper/servicehelper.hxx>
+#include <comphelper/string.hxx>
 #include <comphelper/lok.hxx>
 #include <i18nlangtag/languagetag.hxx>
 #include <cppu/unotype.hxx>
@@ -1407,12 +1408,11 @@ rtl::Reference< ChildAccess > Access::getChild(OUString const & name) {
             // xml:lang attributes, look for the first entry with the same first
             // segment as the requested language tag before falling back to
             // defaults (see fdo#33638):
-            if (aFallbacks.size() > 0)
-                locale = aFallbacks[aFallbacks.size() - 1];
-            assert(
-                !locale.isEmpty() && locale.indexOf('-') == -1 &&
-                locale.indexOf('_') == -1);
-
+            auto const i = comphelper::string::indexOfAny(locale, u"-_", 1);
+            if (i != -1) {
+                locale = locale.copy(0, i);
+            }
+            assert(!locale.isEmpty());
             std::vector< rtl::Reference< ChildAccess > > children(
                 getAllChildren());
             for (auto const& child : children)