configmgr/qa/unit/test.cxx | 7 +++++++
configmgr/source/access.cxx | 12 ++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
New commits:
commit c3bd52f81bf733a0b9b0560794a54b2ac1e0f444
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Wed Feb 15 21:22:51 2023 +0100
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Wed Feb 15 23:24:12 2023 +0000
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 <sberg...@redhat.com>
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)
