Fix aapt2 pseudo-translating donottranslate* files
Bug: 126423638
Test: After building android doing this in the 'out' folder:
Test:
Test: ./soong/host/linux-x86/bin/aapt d --values resources \
Test: ./target/product/sailfish/system/product/priv-app/SystemUIGoogle/SystemUIGoogle.apk \
Test: | less
Test:
Test: search for `system_ui_aod_date_pattern` in the output, found this:
Test: (string8) "[éééḾḾḾð one two]"
Test: (string8) "<U+200F><U+202E>eeeMMMd<U+202C><U+200F>"
Test: (the en-XA and ar-XB pseudo-translated versions of the string)
Test:
Test: After the fix and rebuild the dump only finds the original English string ("eeeMMMd")
Test: Also flashed the image, switched to en-XA, and left the phone around for more than 24 hours.
Change-Id: I2fb7c5b5ee7d3d3200410593346682ed16559056
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index 5336141..8577921 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -217,6 +217,29 @@
EXPECT_THAT(str->value->spans[1].last_char, Eq(13u));
}
+TEST_F(ResourceParserTest, ParseStringTranslatableAttribute) {
+ // If there is no translate attribute the default is 'true'
+ EXPECT_TRUE(TestParse(R"(<string name="foo1">Translate</string>)"));
+ String* str = test::GetValue<String>(&table_, "string/foo1");
+ ASSERT_THAT(str, NotNull());
+ ASSERT_TRUE(str->IsTranslatable());
+
+ // Explicit 'true' translate attribute
+ EXPECT_TRUE(TestParse(R"(<string name="foo2" translatable="true">Translate</string>)"));
+ str = test::GetValue<String>(&table_, "string/foo2");
+ ASSERT_THAT(str, NotNull());
+ ASSERT_TRUE(str->IsTranslatable());
+
+ // Explicit 'false' translate attribute
+ EXPECT_TRUE(TestParse(R"(<string name="foo3" translatable="false">Do not translate</string>)"));
+ str = test::GetValue<String>(&table_, "string/foo3");
+ ASSERT_THAT(str, NotNull());
+ ASSERT_FALSE(str->IsTranslatable());
+
+ // Invalid value for the translate attribute, should be boolean ('true' or 'false')
+ EXPECT_FALSE(TestParse(R"(<string name="foo4" translatable="yes">Translate</string>)"));
+}
+
TEST_F(ResourceParserTest, IgnoreXliffTagsOtherThanG) {
std::string input = R"(
<string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp
index 2ec1ab3..9b81369f 100644
--- a/tools/aapt2/cmd/Compile.cpp
+++ b/tools/aapt2/cmd/Compile.cpp
@@ -143,6 +143,8 @@
const ResourcePathData& path_data, io::IFile* file, IArchiveWriter* writer,
const std::string& output_path) {
TRACE_CALL();
+ // Filenames starting with "donottranslate" are not localizable
+ bool translatable_file = path_data.name.find("donottranslate") != 0;
ResourceTable table;
{
auto fin = file->OpenInputStream();
@@ -157,9 +159,7 @@
ResourceParserOptions parser_options;
parser_options.error_on_positional_arguments = !options.legacy_mode;
-
- // If the filename includes donottranslate, then the default translatable is false.
- parser_options.translatable = path_data.name.find("donottranslate") == std::string::npos;
+ parser_options.translatable = translatable_file;
// If visibility was forced, we need to use it when creating a new resource and also error if
// we try to parse the <public>, <public-group>, <java-symbol> or <symbol> tags.
@@ -172,7 +172,7 @@
}
}
- if (options.pseudolocalize) {
+ if (options.pseudolocalize && translatable_file) {
// Generate pseudo-localized strings (en-XA and ar-XB).
// These are created as weak symbols, and are only generated from default
// configuration
diff --git a/tools/aapt2/cmd/Compile_test.cpp b/tools/aapt2/cmd/Compile_test.cpp
index c0c05cd..5f637bd 100644
--- a/tools/aapt2/cmd/Compile_test.cpp
+++ b/tools/aapt2/cmd/Compile_test.cpp
@@ -27,6 +27,8 @@
namespace aapt {
+using CompilerTest = CommandTestFixture;
+
std::string BuildPath(std::vector<std::string> args) {
std::string out;
if (args.empty()) {
@@ -51,7 +53,7 @@
return CompileCommand(&diag).Execute(args, &std::cerr);
}
-TEST(CompilerTest, MultiplePeriods) {
+TEST_F(CompilerTest, MultiplePeriods) {
StdErrDiagnostics diag;
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
const std::string kResDir = BuildPath({android::base::Dirname(android::base::GetExecutablePath()),
@@ -108,7 +110,7 @@
ASSERT_EQ(::android::base::utf8::unlink(path5_out.c_str()), 0);
}
-TEST(CompilerTest, DirInput) {
+TEST_F(CompilerTest, DirInput) {
StdErrDiagnostics diag;
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
const std::string kResDir = BuildPath({android::base::Dirname(android::base::GetExecutablePath()),
@@ -138,7 +140,7 @@
ASSERT_EQ(::android::base::utf8::unlink(kOutputFlata.c_str()), 0);
}
-TEST(CompilerTest, ZipInput) {
+TEST_F(CompilerTest, ZipInput) {
StdErrDiagnostics diag;
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
const std::string kResZip =
@@ -169,4 +171,86 @@
ASSERT_EQ(::android::base::utf8::unlink(kOutputFlata.c_str()), 0);
}
-} // namespace aapt
\ No newline at end of file
+/*
+ * This tests the "protection" from pseudo-translation of
+ * non-translatable files (starting with 'donotranslate')
+ * and strings (with the translatable="false" attribute)
+ *
+ * We check 4 string files, 2 translatable, and 2 not (based on file name)
+ * Each file contains 2 strings, one translatable, one not (attribute based)
+ * Each of these files are compiled and linked into one .apk, then we load the
+ * strings from the apk and check if there are pseudo-translated strings.
+ */
+
+// Using 000 and 111 because they are not changed by pseudo-translation,
+// making our life easier.
+constexpr static const char sTranslatableXmlContent[] =
+ "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
+ "<resources>"
+ " <string name=\"normal\">000</string>"
+ " <string name=\"non_translatable\" translatable=\"false\">111</string>"
+ "</resources>";
+
+static void AssertTranslations(CommandTestFixture *ctf, std::string file_name,
+ std::vector<std::string> expected) {
+
+ StdErrDiagnostics diag;
+
+ const std::string source_file = ctf->GetTestPath("/res/values/" + file_name + ".xml");
+ const std::string compiled_files_dir = ctf->GetTestPath("/compiled_" + file_name);
+ const std::string out_apk = ctf->GetTestPath("/" + file_name + ".apk");
+
+ CHECK(ctf->WriteFile(source_file, sTranslatableXmlContent));
+ CHECK(file::mkdirs(compiled_files_dir.data()));
+
+ ASSERT_EQ(CompileCommand(&diag).Execute({
+ source_file,
+ "-o", compiled_files_dir,
+ "-v",
+ "--pseudo-localize"
+ }, &std::cerr), 0);
+
+ ASSERT_TRUE(ctf->Link({
+ "--manifest", ctf->GetDefaultManifest(),
+ "-o", out_apk
+ }, compiled_files_dir, &diag));
+
+ std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
+
+ ResourceTable* table = apk->GetResourceTable();
+ ASSERT_NE(table, nullptr);
+ table->string_pool.Sort();
+
+ const std::vector<std::unique_ptr<StringPool::Entry>>& pool_strings =
+ table->string_pool.strings();
+
+ // The actual / expected vectors have the same size
+ const size_t pool_size = pool_strings.size();
+ ASSERT_EQ(pool_size, expected.size());
+
+ for (size_t i = 0; i < pool_size; i++) {
+ std::string actual = pool_strings[i]->value;
+ ASSERT_EQ(actual, expected[i]);
+ }
+}
+
+TEST_F(CompilerTest, DoNotTranslateTest) {
+ // The first string (000) is translatable, the second is not
+ // ar-XB uses "\u200F\u202E...\u202C\u200F"
+ std::vector<std::string> expected_translatable = {
+ "000", "111", // default locale
+ "[000 one]", // en-XA
+ "\xE2\x80\x8F\xE2\x80\xAE" "000" "\xE2\x80\xAC\xE2\x80\x8F", // ar-XB
+ };
+ AssertTranslations(this, "foo", expected_translatable);
+ AssertTranslations(this, "foo_donottranslate", expected_translatable);
+
+ // No translatable strings because these are non-translatable files
+ std::vector<std::string> expected_not_translatable = {
+ "000", "111", // default locale
+ };
+ AssertTranslations(this, "donottranslate", expected_not_translatable);
+ AssertTranslations(this, "donottranslate_foo", expected_not_translatable);
+}
+
+} // namespace aapt