AAPT2: Fix pseudolocalization to respect <xliff:g>
The XLIFF 'g' tag specifies content that should NOT be translated.
AAPT2's pseudolocalization process should respect it.
Bug:34064599
Test: make libandroidfw_tests
Change-Id: Ice437d7f0ff246730ee04896fd035e2d846148fb
diff --git a/libs/androidfw/include/androidfw/StringPiece.h b/libs/androidfw/include/androidfw/StringPiece.h
index c9effd1..02cc505 100644
--- a/libs/androidfw/include/androidfw/StringPiece.h
+++ b/libs/androidfw/include/androidfw/StringPiece.h
@@ -261,8 +261,36 @@
return out.write(str.data(), str.size());
}
+template <typename TChar>
+inline ::std::basic_string<TChar>& operator+=(::std::basic_string<TChar>& lhs,
+ const BasicStringPiece<TChar>& rhs) {
+ return lhs.append(rhs.data(), rhs.size());
+}
+
+template <typename TChar>
+inline bool operator==(const ::std::basic_string<TChar>& lhs, const BasicStringPiece<TChar>& rhs) {
+ return rhs == lhs;
+}
+
+template <typename TChar>
+inline bool operator!=(const ::std::basic_string<TChar>& lhs, const BasicStringPiece<TChar>& rhs) {
+ return rhs != lhs;
+}
+
} // namespace android
+inline ::std::ostream& operator<<(::std::ostream& out, const std::u16string& str) {
+ ssize_t utf8_len = utf16_to_utf8_length(str.data(), str.size());
+ if (utf8_len < 0) {
+ return out << "???";
+ }
+
+ std::string utf8;
+ utf8.resize(static_cast<size_t>(utf8_len));
+ utf16_to_utf8(str.data(), str.size(), &*utf8.begin(), utf8_len + 1);
+ return out << utf8;
+}
+
namespace std {
template <typename TChar>
diff --git a/tools/aapt2/Main.cpp b/tools/aapt2/Main.cpp
index 227ffa3..ba378d7 100644
--- a/tools/aapt2/Main.cpp
+++ b/tools/aapt2/Main.cpp
@@ -25,7 +25,7 @@
static const char* sMajorVersion = "2";
// Update minor version whenever a feature or flag is added.
-static const char* sMinorVersion = "6";
+static const char* sMinorVersion = "7";
int PrintVersion() {
std::cerr << "Android Asset Packaging Tool (aapt) " << sMajorVersion << "."
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 1c750c6..47ca266 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -26,6 +26,7 @@
#include "ResourceValues.h"
#include "ValueVisitor.h"
#include "util/ImmutableMap.h"
+#include "util/Maybe.h"
#include "util/Util.h"
#include "xml/XmlPullParser.h"
@@ -150,82 +151,108 @@
/**
* Build a string from XML that converts nested elements into Span objects.
*/
-bool ResourceParser::FlattenXmlSubtree(xml::XmlPullParser* parser,
- std::string* out_raw_string,
- StyleString* out_style_string) {
+bool ResourceParser::FlattenXmlSubtree(
+ xml::XmlPullParser* parser, std::string* out_raw_string, StyleString* out_style_string,
+ std::vector<UntranslatableSection>* out_untranslatable_sections) {
+ // Keeps track of formatting tags (<b>, <i>) and the range of characters for which they apply.
std::vector<Span> span_stack;
- bool error = false;
+ // Clear the output variables.
out_raw_string->clear();
out_style_string->spans.clear();
+ out_untranslatable_sections->clear();
+
+ // The StringBuilder will concatenate the various segments of text which are initially
+ // separated by tags. It also handles unicode escape codes and quotations.
util::StringBuilder builder;
+
+ // The first occurrence of a <xliff:g> tag. Nested <xliff:g> tags are illegal.
+ Maybe<size_t> untranslatable_start_depth;
+
size_t depth = 1;
while (xml::XmlPullParser::IsGoodEvent(parser->Next())) {
const xml::XmlPullParser::Event event = parser->event();
- if (event == xml::XmlPullParser::Event::kEndElement) {
- if (!parser->element_namespace().empty()) {
- // We already warned and skipped the start element, so just skip here
- // too
- continue;
+
+ if (event == xml::XmlPullParser::Event::kStartElement) {
+ if (parser->element_namespace().empty()) {
+ // This is an HTML tag which we encode as a span. Add it to the span stack.
+ std::string span_name = parser->element_name();
+ const auto end_attr_iter = parser->end_attributes();
+ for (auto attr_iter = parser->begin_attributes(); attr_iter != end_attr_iter; ++attr_iter) {
+ span_name += ";";
+ span_name += attr_iter->name;
+ span_name += "=";
+ span_name += attr_iter->value;
+ }
+
+ // Make sure the string is representable in our binary format.
+ if (builder.Utf16Len() > std::numeric_limits<uint32_t>::max()) {
+ diag_->Error(DiagMessage(source_.WithLine(parser->line_number()))
+ << "style string '" << builder.ToString() << "' is too long");
+ return false;
+ }
+
+ span_stack.push_back(Span{std::move(span_name), static_cast<uint32_t>(builder.Utf16Len())});
+ } else if (parser->element_namespace() == sXliffNamespaceUri) {
+ if (parser->element_name() == "g") {
+ if (untranslatable_start_depth) {
+ // We've already encountered an <xliff:g> tag, and nested <xliff:g> tags are illegal.
+ diag_->Error(DiagMessage(source_.WithLine(parser->line_number()))
+ << "illegal nested XLIFF 'g' tag");
+ return false;
+ } else {
+ // Mark the start of an untranslatable section. Use UTF8 indices/lengths.
+ untranslatable_start_depth = depth;
+ const size_t current_idx = builder.ToString().size();
+ out_untranslatable_sections->push_back(UntranslatableSection{current_idx, current_idx});
+ }
+ }
+ // Ignore other xliff tags, they get handled by other tools.
+
+ } else {
+ // Besides XLIFF, any other namespaced tag is unsupported and ignored.
+ diag_->Warn(DiagMessage(source_.WithLine(parser->line_number()))
+ << "ignoring element '" << parser->element_name()
+ << "' with unknown namespace '" << parser->element_namespace() << "'");
}
+ // Enter one level inside the element.
+ depth++;
+ } else if (event == xml::XmlPullParser::Event::kText) {
+ // Record both the raw text and append to the builder to deal with escape sequences
+ // and quotations.
+ out_raw_string->append(parser->text());
+ builder.Append(parser->text());
+ } else if (event == xml::XmlPullParser::Event::kEndElement) {
+ // Return one level from within the element.
depth--;
if (depth == 0) {
break;
}
- span_stack.back().last_char = builder.Utf16Len() - 1;
- out_style_string->spans.push_back(span_stack.back());
- span_stack.pop_back();
-
- } else if (event == xml::XmlPullParser::Event::kText) {
- out_raw_string->append(parser->text());
- builder.Append(parser->text());
-
- } else if (event == xml::XmlPullParser::Event::kStartElement) {
- if (!parser->element_namespace().empty()) {
- if (parser->element_namespace() != sXliffNamespaceUri) {
- // Only warn if this isn't an xliff namespace.
- diag_->Warn(DiagMessage(source_.WithLine(parser->line_number()))
- << "skipping element '" << parser->element_name()
- << "' with unknown namespace '"
- << parser->element_namespace() << "'");
- }
- continue;
+ if (parser->element_namespace().empty()) {
+ // This is an HTML tag which we encode as a span. Update the span
+ // stack and pop the top entry.
+ Span& top_span = span_stack.back();
+ top_span.last_char = builder.Utf16Len() - 1;
+ out_style_string->spans.push_back(std::move(top_span));
+ span_stack.pop_back();
+ } else if (untranslatable_start_depth == make_value(depth)) {
+ // This is the end of an untranslatable section. Use UTF8 indices/lengths.
+ UntranslatableSection& untranslatable_section = out_untranslatable_sections->back();
+ untranslatable_section.end = builder.ToString().size();
+ untranslatable_start_depth = {};
}
- depth++;
-
- // Build a span object out of the nested element.
- std::string span_name = parser->element_name();
- const auto end_attr_iter = parser->end_attributes();
- for (auto attr_iter = parser->begin_attributes();
- attr_iter != end_attr_iter; ++attr_iter) {
- span_name += ";";
- span_name += attr_iter->name;
- span_name += "=";
- span_name += attr_iter->value;
- }
-
- if (builder.Utf16Len() > std::numeric_limits<uint32_t>::max()) {
- diag_->Error(DiagMessage(source_.WithLine(parser->line_number()))
- << "style string '" << builder.ToString()
- << "' is too long");
- error = true;
- } else {
- span_stack.push_back(
- Span{span_name, static_cast<uint32_t>(builder.Utf16Len())});
- }
-
} else if (event == xml::XmlPullParser::Event::kComment) {
- // Skip
+ // Ignore.
} else {
LOG(FATAL) << "unhandled XML event";
}
}
- CHECK(span_stack.empty()) << "spans haven't been fully processed";
+ CHECK(span_stack.empty()) << "spans haven't been fully processed";
out_style_string->str = builder.ToString();
- return !error;
+ return true;
}
bool ResourceParser::Parse(xml::XmlPullParser* parser) {
@@ -548,15 +575,18 @@
std::string raw_value;
StyleString style_string;
- if (!FlattenXmlSubtree(parser, &raw_value, &style_string)) {
+ std::vector<UntranslatableSection> untranslatable_sections;
+ if (!FlattenXmlSubtree(parser, &raw_value, &style_string, &untranslatable_sections)) {
return {};
}
if (!style_string.spans.empty()) {
// This can only be a StyledString.
- return util::make_unique<StyledString>(table_->string_pool.MakeRef(
- style_string,
- StringPool::Context(StringPool::Context::kStylePriority, config_)));
+ std::unique_ptr<StyledString> styled_string =
+ util::make_unique<StyledString>(table_->string_pool.MakeRef(
+ style_string, StringPool::Context(StringPool::Context::kStylePriority, config_)));
+ styled_string->untranslatable_sections = std::move(untranslatable_sections);
+ return std::move(styled_string);
}
auto on_create_reference = [&](const ResourceName& name) {
@@ -582,8 +612,10 @@
// Try making a regular string.
if (type_mask & android::ResTable_map::TYPE_STRING) {
// Use the trimmed, escaped string.
- return util::make_unique<String>(table_->string_pool.MakeRef(
- style_string.str, StringPool::Context(config_)));
+ std::unique_ptr<String> string = util::make_unique<String>(
+ table_->string_pool.MakeRef(style_string.str, StringPool::Context(config_)));
+ string->untranslatable_sections = std::move(untranslatable_sections);
+ return std::move(string);
}
if (allow_raw_value) {
@@ -609,17 +641,15 @@
formatted = maybe_formatted.value();
}
- bool translateable = options_.translatable;
- if (Maybe<StringPiece> translateable_attr =
- xml::FindAttribute(parser, "translatable")) {
- Maybe<bool> maybe_translateable =
- ResourceUtils::ParseBool(translateable_attr.value());
- if (!maybe_translateable) {
+ bool translatable = options_.translatable;
+ if (Maybe<StringPiece> translatable_attr = xml::FindAttribute(parser, "translatable")) {
+ Maybe<bool> maybe_translatable = ResourceUtils::ParseBool(translatable_attr.value());
+ if (!maybe_translatable) {
diag_->Error(DiagMessage(out_resource->source)
<< "invalid value for 'translatable'. Must be a boolean");
return false;
}
- translateable = maybe_translateable.value();
+ translatable = maybe_translatable.value();
}
out_resource->value =
@@ -630,9 +660,9 @@
}
if (String* string_value = ValueCast<String>(out_resource->value.get())) {
- string_value->SetTranslateable(translateable);
+ string_value->SetTranslatable(translatable);
- if (formatted && translateable) {
+ if (formatted && translatable) {
if (!util::VerifyJavaStringFormat(*string_value->value)) {
DiagMessage msg(out_resource->source);
msg << "multiple substitutions specified in non-positional format; "
@@ -646,9 +676,8 @@
}
}
- } else if (StyledString* string_value =
- ValueCast<StyledString>(out_resource->value.get())) {
- string_value->SetTranslateable(translateable);
+ } else if (StyledString* string_value = ValueCast<StyledString>(out_resource->value.get())) {
+ string_value->SetTranslatable(translatable);
}
return true;
}
@@ -1151,19 +1180,17 @@
std::unique_ptr<Array> array = util::make_unique<Array>();
- bool translateable = options_.translatable;
- if (Maybe<StringPiece> translateable_attr =
- xml::FindAttribute(parser, "translatable")) {
- Maybe<bool> maybe_translateable =
- ResourceUtils::ParseBool(translateable_attr.value());
- if (!maybe_translateable) {
+ bool translatable = options_.translatable;
+ if (Maybe<StringPiece> translatable_attr = xml::FindAttribute(parser, "translatable")) {
+ Maybe<bool> maybe_translatable = ResourceUtils::ParseBool(translatable_attr.value());
+ if (!maybe_translatable) {
diag_->Error(DiagMessage(out_resource->source)
<< "invalid value for 'translatable'. Must be a boolean");
return false;
}
- translateable = maybe_translateable.value();
+ translatable = maybe_translatable.value();
}
- array->SetTranslateable(translateable);
+ array->SetTranslatable(translatable);
bool error = false;
const size_t depth = parser->depth();
diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h
index cc0fa26..8258019 100644
--- a/tools/aapt2/ResourceParser.h
+++ b/tools/aapt2/ResourceParser.h
@@ -60,16 +60,16 @@
private:
DISALLOW_COPY_AND_ASSIGN(ResourceParser);
- /*
- * Parses the XML subtree as a StyleString (flattened XML representation for
- * strings
- * with formatting). If successful, `out_style_string`
- * contains the escaped and whitespace trimmed text, while `out_raw_string`
- * contains the unescaped text. Returns true on success.
- */
- bool FlattenXmlSubtree(xml::XmlPullParser* parser,
- std::string* out_raw_string,
- StyleString* out_style_string);
+ // Parses the XML subtree as a StyleString (flattened XML representation for strings with
+ // formatting). If parsing fails, false is returned and the out parameters are left in an
+ // unspecified state. Otherwise,
+ // `out_style_string` contains the escaped and whitespace trimmed text.
+ // `out_raw_string` contains the un-escaped text.
+ // `out_untranslatable_sections` contains the sections of the string that should not be
+ // translated.
+ bool FlattenXmlSubtree(xml::XmlPullParser* parser, std::string* out_raw_string,
+ StyleString* out_style_string,
+ std::vector<UntranslatableSection>* out_untranslatable_sections);
/*
* Parses the XML subtree and returns an Item.
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index cf901da..67ed476 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -76,6 +76,7 @@
String* str = test::GetValue<String>(&table_, "string/foo");
ASSERT_NE(nullptr, str);
EXPECT_EQ(std::string(" hey there "), *str->value);
+ EXPECT_TRUE(str->untranslatable_sections.empty());
}
TEST_F(ResourceParserTest, ParseEscapedString) {
@@ -85,6 +86,7 @@
String* str = test::GetValue<String>(&table_, "string/foo");
ASSERT_NE(nullptr, str);
EXPECT_EQ(std::string("?123"), *str->value);
+ EXPECT_TRUE(str->untranslatable_sections.empty());
}
TEST_F(ResourceParserTest, ParseFormattedString) {
@@ -97,8 +99,7 @@
TEST_F(ResourceParserTest, ParseStyledString) {
// Use a surrogate pair unicode point so that we can verify that the span
- // indices
- // use UTF-16 length and not UTF-18 length.
+ // indices use UTF-16 length and not UTF-8 length.
std::string input =
"<string name=\"foo\">This is my aunt\u2019s <b>string</b></string>";
ASSERT_TRUE(TestParse(input));
@@ -109,6 +110,7 @@
const std::string expected_str = "This is my aunt\u2019s string";
EXPECT_EQ(expected_str, *str->value->str);
EXPECT_EQ(1u, str->value->spans.size());
+ EXPECT_TRUE(str->untranslatable_sections.empty());
EXPECT_EQ(std::string("b"), *str->value->spans[0].name);
EXPECT_EQ(17u, str->value->spans[0].first_char);
@@ -122,6 +124,7 @@
String* str = test::GetValue<String>(&table_, "string/foo");
ASSERT_NE(nullptr, str);
EXPECT_EQ(std::string("This is what I think"), *str->value);
+ EXPECT_TRUE(str->untranslatable_sections.empty());
input = "<string name=\"foo2\">\" This is what I think \"</string>";
ASSERT_TRUE(TestParse(input));
@@ -131,16 +134,61 @@
EXPECT_EQ(std::string(" This is what I think "), *str->value);
}
-TEST_F(ResourceParserTest, IgnoreXliffTags) {
- std::string input =
- "<string name=\"foo\" \n"
- " xmlns:xliff=\"urn:oasis:names:tc:xliff:document:1.2\">\n"
- " There are <xliff:g id=\"count\">%1$d</xliff:g> apples</string>";
+TEST_F(ResourceParserTest, IgnoreXliffTagsOtherThanG) {
+ std::string input = R"EOF(
+ <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
+ There are <xliff:source>no</xliff:source> apples</string>)EOF";
+ ASSERT_TRUE(TestParse(input));
+
+ String* str = test::GetValue<String>(&table_, "string/foo");
+ ASSERT_NE(nullptr, str);
+ EXPECT_EQ(StringPiece("There are no apples"), StringPiece(*str->value));
+ EXPECT_TRUE(str->untranslatable_sections.empty());
+}
+
+TEST_F(ResourceParserTest, NestedXliffGTagsAreIllegal) {
+ std::string input = R"EOF(
+ <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
+ Do not <xliff:g>translate <xliff:g>this</xliff:g></xliff:g></string>)EOF";
+ EXPECT_FALSE(TestParse(input));
+}
+
+TEST_F(ResourceParserTest, RecordUntranslateableXliffSectionsInString) {
+ std::string input = R"EOF(
+ <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
+ There are <xliff:g id="count">%1$d</xliff:g> apples</string>)EOF";
ASSERT_TRUE(TestParse(input));
String* str = test::GetValue<String>(&table_, "string/foo");
ASSERT_NE(nullptr, str);
EXPECT_EQ(StringPiece("There are %1$d apples"), StringPiece(*str->value));
+
+ ASSERT_EQ(1u, str->untranslatable_sections.size());
+
+ // We expect indices and lengths that span to include the whitespace
+ // before %1$d. This is due to how the StringBuilder withholds whitespace unless
+ // needed (to deal with line breaks, etc.).
+ EXPECT_EQ(9u, str->untranslatable_sections[0].start);
+ EXPECT_EQ(14u, str->untranslatable_sections[0].end);
+}
+
+TEST_F(ResourceParserTest, RecordUntranslateableXliffSectionsInStyledString) {
+ std::string input = R"EOF(
+ <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
+ There are <b><xliff:g id="count">%1$d</xliff:g></b> apples</string>)EOF";
+ ASSERT_TRUE(TestParse(input));
+
+ StyledString* str = test::GetValue<StyledString>(&table_, "string/foo");
+ ASSERT_NE(nullptr, str);
+ EXPECT_EQ(StringPiece("There are %1$d apples"), StringPiece(*str->value->str));
+
+ ASSERT_EQ(1u, str->untranslatable_sections.size());
+
+ // We expect indices and lengths that span to include the whitespace
+ // before %1$d. This is due to how the StringBuilder withholds whitespace unless
+ // needed (to deal with line breaks, etc.).
+ EXPECT_EQ(9u, str->untranslatable_sections[0].start);
+ EXPECT_EQ(14u, str->untranslatable_sections[0].end);
}
TEST_F(ResourceParserTest, ParseNull) {
@@ -149,15 +197,11 @@
// The Android runtime treats a value of android::Res_value::TYPE_NULL as
// a non-existing value, and this causes problems in styles when trying to
- // resolve
- // an attribute. Null values must be encoded as
- // android::Res_value::TYPE_REFERENCE
+ // resolve an attribute. Null values must be encoded as android::Res_value::TYPE_REFERENCE
// with a data value of 0.
- BinaryPrimitive* integer =
- test::GetValue<BinaryPrimitive>(&table_, "integer/foo");
+ BinaryPrimitive* integer = test::GetValue<BinaryPrimitive>(&table_, "integer/foo");
ASSERT_NE(nullptr, integer);
- EXPECT_EQ(uint16_t(android::Res_value::TYPE_REFERENCE),
- integer->value.dataType);
+ EXPECT_EQ(uint16_t(android::Res_value::TYPE_REFERENCE), integer->value.dataType);
EXPECT_EQ(0u, integer->value.data);
}
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 7956ad8..f75ed7a 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -140,7 +140,23 @@
if (!other) {
return false;
}
- return *this->value == *other->value;
+
+ if (this->value != other->value) {
+ return false;
+ }
+
+ if (untranslatable_sections.size() != other->untranslatable_sections.size()) {
+ return false;
+ }
+
+ auto other_iter = other->untranslatable_sections.begin();
+ for (const UntranslatableSection& this_section : untranslatable_sections) {
+ if (this_section != *other_iter) {
+ return false;
+ }
+ ++other_iter;
+ }
+ return true;
}
bool String::Flatten(android::Res_value* out_value) const {
@@ -158,6 +174,7 @@
String* str = new String(new_pool->MakeRef(*value));
str->comment_ = comment_;
str->source_ = source_;
+ str->untranslatable_sections = untranslatable_sections;
return str;
}
@@ -173,17 +190,22 @@
return false;
}
- if (*this->value->str == *other->value->str) {
- const std::vector<StringPool::Span>& spans_a = this->value->spans;
- const std::vector<StringPool::Span>& spans_b = other->value->spans;
- return std::equal(
- spans_a.begin(), spans_a.end(), spans_b.begin(),
- [](const StringPool::Span& a, const StringPool::Span& b) -> bool {
- return *a.name == *b.name && a.first_char == b.first_char &&
- a.last_char == b.last_char;
- });
+ if (this->value != other->value) {
+ return false;
}
- return false;
+
+ if (untranslatable_sections.size() != other->untranslatable_sections.size()) {
+ return false;
+ }
+
+ auto other_iter = other->untranslatable_sections.begin();
+ for (const UntranslatableSection& this_section : untranslatable_sections) {
+ if (this_section != *other_iter) {
+ return false;
+ }
+ ++other_iter;
+ }
+ return true;
}
bool StyledString::Flatten(android::Res_value* out_value) const {
@@ -200,6 +222,7 @@
StyledString* str = new StyledString(new_pool->MakeRef(value));
str->comment_ = comment_;
str->source_ = source_;
+ str->untranslatable_sections = untranslatable_sections;
return str;
}
@@ -218,7 +241,7 @@
if (!other) {
return false;
}
- return *path == *other->path;
+ return path == other->path;
}
bool FileReference::Flatten(android::Res_value* out_value) const {
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index d380f8d..c71c738 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -34,44 +34,35 @@
struct RawValueVisitor;
-/**
- * A resource value. This is an all-encompassing representation
- * of Item and Map and their subclasses. The way to do
- * type specific operations is to check the Value's type() and
- * cast it to the appropriate subclass. This isn't super clean,
- * but it is the simplest strategy.
- */
+// A resource value. This is an all-encompassing representation
+// of Item and Map and their subclasses. The way to do
+// type specific operations is to check the Value's type() and
+// cast it to the appropriate subclass. This isn't super clean,
+// but it is the simplest strategy.
struct Value {
virtual ~Value() = default;
- /**
- * Whether this value is weak and can be overridden without
- * warning or error. Default is false.
- */
+ // Whether this value is weak and can be overridden without warning or error. Default is false.
bool IsWeak() const { return weak_; }
void SetWeak(bool val) { weak_ = val; }
- // Whether the value is marked as translateable.
+ // Whether the value is marked as translatable.
// This does not persist when flattened.
// It is only used during compilation phase.
- void SetTranslateable(bool val) { translateable_ = val; }
+ void SetTranslatable(bool val) { translatable_ = val; }
// Default true.
- bool IsTranslateable() const { return translateable_; }
+ bool IsTranslatable() const { return translatable_; }
- /**
- * Returns the source where this value was defined.
- */
+ // Returns the source where this value was defined.
const Source& GetSource() const { return source_; }
void SetSource(const Source& source) { source_ = source; }
void SetSource(Source&& source) { source_ = std::move(source); }
- /**
- * Returns the comment that was associated with this resource.
- */
+ // Returns the comment that was associated with this resource.
const std::string& GetComment() const { return comment_; }
void SetComment(const android::StringPiece& str) { comment_ = str.to_string(); }
@@ -80,70 +71,48 @@
virtual bool Equals(const Value* value) const = 0;
- /**
- * Calls the appropriate overload of ValueVisitor.
- */
+ // Calls the appropriate overload of ValueVisitor.
virtual void Accept(RawValueVisitor* visitor) = 0;
- /**
- * Clone the value. new_pool is the new StringPool that
- * any resources with strings should use when copying their string.
- */
+ // Clone the value. `new_pool` is the new StringPool that
+ // any resources with strings should use when copying their string.
virtual Value* Clone(StringPool* new_pool) const = 0;
- /**
- * Human readable printout of this value.
- */
+ // Human readable printout of this value.
virtual void Print(std::ostream* out) const = 0;
protected:
Source source_;
std::string comment_;
bool weak_ = false;
- bool translateable_ = true;
+ bool translatable_ = true;
};
-/**
- * Inherit from this to get visitor accepting implementations for free.
- */
+// Inherit from this to get visitor accepting implementations for free.
template <typename Derived>
struct BaseValue : public Value {
void Accept(RawValueVisitor* visitor) override;
};
-/**
- * A resource item with a single value. This maps to android::ResTable_entry.
- */
+// A resource item with a single value. This maps to android::ResTable_entry.
struct Item : public Value {
- /**
- * Clone the Item.
- */
+ // Clone the Item.
virtual Item* Clone(StringPool* new_pool) const override = 0;
- /**
- * Fills in an android::Res_value structure with this Item's binary
- * representation.
- * Returns false if an error occurred.
- */
+ // Fills in an android::Res_value structure with this Item's binary representation.
+ // Returns false if an error occurred.
virtual bool Flatten(android::Res_value* out_value) const = 0;
};
-/**
- * Inherit from this to get visitor accepting implementations for free.
- */
+// Inherit from this to get visitor accepting implementations for free.
template <typename Derived>
struct BaseItem : public Item {
void Accept(RawValueVisitor* visitor) override;
};
-/**
- * A reference to another resource. This maps to
- * android::Res_value::TYPE_REFERENCE.
- *
- * A reference can be symbolic (with the name set to a valid resource name) or
- * be
- * numeric (the id is set to a valid resource ID).
- */
+// A reference to another resource. This maps to android::Res_value::TYPE_REFERENCE.
+// A reference can be symbolic (with the name set to a valid resource name) or be
+// numeric (the id is set to a valid resource ID).
struct Reference : public BaseItem<Reference> {
enum class Type {
kResource,
@@ -169,9 +138,7 @@
bool operator<(const Reference&, const Reference&);
bool operator==(const Reference&, const Reference&);
-/**
- * An ID resource. Has no real value, just a place holder.
- */
+// An ID resource. Has no real value, just a place holder.
struct Id : public BaseItem<Id> {
Id() { weak_ = true; }
bool Equals(const Value* value) const override;
@@ -180,11 +147,8 @@
void Print(std::ostream* out) const override;
};
-/**
- * A raw, unprocessed string. This may contain quotations,
- * escape sequences, and whitespace. This shall *NOT*
- * end up in the final resource table.
- */
+// A raw, unprocessed string. This may contain quotations, escape sequences, and whitespace.
+// This shall *NOT* end up in the final resource table.
struct RawString : public BaseItem<RawString> {
StringPool::Ref value;
@@ -196,9 +160,32 @@
void Print(std::ostream* out) const override;
};
+// Identifies a range of characters in a string that are untranslatable.
+// These should not be pseudolocalized. The start and end indices are measured in bytes.
+struct UntranslatableSection {
+ // Start offset inclusive.
+ size_t start;
+
+ // End offset exclusive.
+ size_t end;
+};
+
+inline bool operator==(const UntranslatableSection& a, const UntranslatableSection& b) {
+ return a.start == b.start && a.end == b.end;
+}
+
+inline bool operator!=(const UntranslatableSection& a, const UntranslatableSection& b) {
+ return a.start != b.start || a.end != b.end;
+}
+
struct String : public BaseItem<String> {
StringPool::Ref value;
+ // Sections of the string to NOT translate. Mainly used
+ // for pseudolocalization. This data is NOT persisted
+ // in any format.
+ std::vector<UntranslatableSection> untranslatable_sections;
+
explicit String(const StringPool::Ref& ref);
bool Equals(const Value* value) const override;
@@ -210,6 +197,11 @@
struct StyledString : public BaseItem<StyledString> {
StringPool::StyleRef value;
+ // Sections of the string to NOT translate. Mainly used
+ // for pseudolocalization. This data is NOT persisted
+ // in any format.
+ std::vector<UntranslatableSection> untranslatable_sections;
+
explicit StyledString(const StringPool::StyleRef& ref);
bool Equals(const Value* value) const override;
@@ -221,9 +213,8 @@
struct FileReference : public BaseItem<FileReference> {
StringPool::Ref path;
- /**
- * A handle to the file object from which this file can be read.
- */
+ // A handle to the file object from which this file can be read.
+ // This field is NOT persisted in any format. It is transient.
io::IFile* file = nullptr;
FileReference() = default;
@@ -235,9 +226,7 @@
void Print(std::ostream* out) const override;
};
-/**
- * Represents any other android::Res_value.
- */
+// Represents any other android::Res_value.
struct BinaryPrimitive : public BaseItem<BinaryPrimitive> {
android::Res_value value;
@@ -279,10 +268,7 @@
Maybe<Reference> parent;
- /**
- * If set to true, the parent was auto inferred from the
- * style's name.
- */
+ // If set to true, the parent was auto inferred from the style's name.
bool parent_inferred = false;
std::vector<Entry> entries;
@@ -319,9 +305,7 @@
void MergeWith(Styleable* styleable);
};
-/**
- * Stream operator for printing Value objects.
- */
+// Stream operator for printing Value objects.
inline ::std::ostream& operator<<(::std::ostream& out, const Value& value) {
value.Print(&out);
return out;
diff --git a/tools/aapt2/StringPool.cpp b/tools/aapt2/StringPool.cpp
index d968d73..57da5f0 100644
--- a/tools/aapt2/StringPool.cpp
+++ b/tools/aapt2/StringPool.cpp
@@ -63,6 +63,14 @@
return *this;
}
+bool StringPool::Ref::operator==(const Ref& rhs) const {
+ return entry_->value == rhs.entry_->value;
+}
+
+bool StringPool::Ref::operator!=(const Ref& rhs) const {
+ return entry_->value != rhs.entry_->value;
+}
+
const std::string* StringPool::Ref::operator->() const {
return &entry_->value;
}
@@ -109,6 +117,28 @@
return *this;
}
+bool StringPool::StyleRef::operator==(const StyleRef& rhs) const {
+ if (entry_->str != rhs.entry_->str) {
+ return false;
+ }
+
+ if (entry_->spans.size() != rhs.entry_->spans.size()) {
+ return false;
+ }
+
+ auto rhs_iter = rhs.entry_->spans.begin();
+ for (const Span& span : entry_->spans) {
+ const Span& rhs_span = *rhs_iter;
+ if (span.first_char != rhs_span.first_char || span.last_char != rhs_span.last_char ||
+ span.name != rhs_span.name) {
+ return false;
+ }
+ }
+ return true;
+}
+
+bool StringPool::StyleRef::operator!=(const StyleRef& rhs) const { return !operator==(rhs); }
+
const StringPool::StyleEntry* StringPool::StyleRef::operator->() const {
return entry_;
}
diff --git a/tools/aapt2/StringPool.h b/tools/aapt2/StringPool.h
index d0ce489..a626d37 100644
--- a/tools/aapt2/StringPool.h
+++ b/tools/aapt2/StringPool.h
@@ -70,6 +70,8 @@
~Ref();
Ref& operator=(const Ref& rhs);
+ bool operator==(const Ref& rhs) const;
+ bool operator!=(const Ref& rhs) const;
const std::string* operator->() const;
const std::string& operator*() const;
@@ -93,6 +95,8 @@
~StyleRef();
StyleRef& operator=(const StyleRef& rhs);
+ bool operator==(const StyleRef& rhs) const;
+ bool operator!=(const StyleRef& rhs) const;
const StyleEntry* operator->() const;
const StyleEntry& operator*() const;
diff --git a/tools/aapt2/compile/PseudolocaleGenerator.cpp b/tools/aapt2/compile/PseudolocaleGenerator.cpp
index 5035f81..fad9edd 100644
--- a/tools/aapt2/compile/PseudolocaleGenerator.cpp
+++ b/tools/aapt2/compile/PseudolocaleGenerator.cpp
@@ -43,16 +43,16 @@
}
// The ranges are all represented with a single value. This is the start of
- // one range and
- // end of another.
+ // one range and end of another.
struct Range {
size_t start;
- // Once the new string is localized, these are the pointers to the spans to
- // adjust.
+ // If set to true, toggles the state of translatability.
+ bool toggle_translatability;
+
+ // Once the new string is localized, these are the pointers to the spans to adjust.
// Since this struct represents the start of one range and end of another,
- // we have
- // the two pointers respectively.
+ // we have the two pointers respectively.
uint32_t* update_start;
uint32_t* update_end;
};
@@ -63,12 +63,11 @@
// Construct the ranges. The ranges are represented like so: [0, 2, 5, 7]
// The ranges are the spaces in between. In this example, with a total string
- // length of 9,
- // the vector represents: (0,1], (2,4], (5,6], (7,9]
+ // length of 9, the vector represents: (0,1], (2,4], (5,6], (7,9]
//
std::vector<Range> ranges;
- ranges.push_back(Range{0});
- ranges.push_back(Range{original_text.size() - 1});
+ ranges.push_back(Range{0, false, nullptr, nullptr});
+ ranges.push_back(Range{original_text.size() - 1, false, nullptr, nullptr});
for (size_t i = 0; i < string->value->spans.size(); i++) {
const StringPool::Span& span = string->value->spans[i];
@@ -78,8 +77,7 @@
if (iter != ranges.end() && iter->start == span.first_char) {
iter->update_start = &localized.spans[i].first_char;
} else {
- ranges.insert(iter, Range{span.first_char, &localized.spans[i].first_char,
- nullptr});
+ ranges.insert(iter, Range{span.first_char, false, &localized.spans[i].first_char, nullptr});
}
// Insert or update the Range marker for the end of this span.
@@ -87,14 +85,45 @@
if (iter != ranges.end() && iter->start == span.last_char) {
iter->update_end = &localized.spans[i].last_char;
} else {
- ranges.insert(
- iter, Range{span.last_char, nullptr, &localized.spans[i].last_char});
+ ranges.insert(iter, Range{span.last_char, false, nullptr, &localized.spans[i].last_char});
+ }
+ }
+
+ // Parts of the string may be untranslatable. Merge those ranges
+ // in as well, so that we have continuous sections of text to
+ // feed into the pseudolocalizer.
+ // We do this by marking the beginning of a range as either toggling
+ // the translatability state or not.
+ for (const UntranslatableSection& section : string->untranslatable_sections) {
+ auto iter = std::lower_bound(ranges.begin(), ranges.end(), section.start, cmp);
+ if (iter != ranges.end() && iter->start == section.start) {
+ // An existing span starts (or ends) here. We just need to mark that
+ // the translatability should toggle here. If translatability was
+ // already being toggled, then that means we have two adjacent ranges of untranslatable
+ // text, so remove the toggle and only toggle at the end of this range,
+ // effectively merging these ranges.
+ iter->toggle_translatability = !iter->toggle_translatability;
+ } else {
+ // Insert a new range that specifies to toggle the translatability.
+ iter = ranges.insert(iter, Range{section.start, true, nullptr, nullptr});
+ }
+
+ // Update/create an end to the untranslatable section.
+ iter = std::lower_bound(iter, ranges.end(), section.end, cmp);
+ if (iter != ranges.end() && iter->start == section.end) {
+ iter->toggle_translatability = true;
+ } else {
+ iter = ranges.insert(iter, Range{section.end, true, nullptr, nullptr});
}
}
localized.str += localizer.Start();
// Iterate over the ranges and localize each section.
+ // The text starts as translatable, and each time a range has toggle_translatability
+ // set to true, we toggle whether to translate or not.
+ // This assumes no untranslatable ranges overlap.
+ bool translatable = true;
for (size_t i = 0; i < ranges.size(); i++) {
const size_t start = ranges[i].start;
size_t len = original_text.size() - start;
@@ -110,15 +139,20 @@
*ranges[i].update_end = localized.str.size();
}
- localized.str += localizer.Text(original_text.substr(start, len));
+ if (ranges[i].toggle_translatability) {
+ translatable = !translatable;
+ }
+
+ if (translatable) {
+ localized.str += localizer.Text(original_text.substr(start, len));
+ } else {
+ localized.str += original_text.substr(start, len);
+ }
}
localized.str += localizer.End();
- std::unique_ptr<StyledString> localized_string =
- util::make_unique<StyledString>(pool->MakeRef(localized));
- localized_string->SetSource(string->GetSource());
- return localized_string;
+ return util::make_unique<StyledString>(pool->MakeRef(localized));
}
namespace {
@@ -152,8 +186,30 @@
}
void Visit(String* string) override {
- std::string result =
- localizer_.Start() + localizer_.Text(*string->value) + localizer_.End();
+ const StringPiece original_string = *string->value;
+ std::string result = localizer_.Start();
+
+ // Pseudolocalize only the translatable sections.
+ size_t start = 0u;
+ for (const UntranslatableSection& section : string->untranslatable_sections) {
+ // Pseudolocalize the content before the untranslatable section.
+ const size_t len = section.start - start;
+ if (len > 0u) {
+ result += localizer_.Text(original_string.substr(start, len));
+ }
+
+ // Copy the untranslatable content.
+ result += original_string.substr(section.start, section.end - section.start);
+ start = section.end;
+ }
+
+ // Pseudolocalize the content after the last untranslatable section.
+ if (start != original_string.size()) {
+ const size_t len = original_string.size() - start;
+ result += localizer_.Text(original_string.substr(start, len));
+ }
+ result += localizer_.End();
+
std::unique_ptr<String> localized =
util::make_unique<String>(pool_->MakeRef(result));
localized->SetSource(string->GetSource());
@@ -163,6 +219,7 @@
void Visit(StyledString* string) override {
item = PseudolocalizeStyledString(string, method_, pool_);
+ item->SetSource(string->GetSource());
item->SetWeak(true);
}
@@ -228,7 +285,7 @@
/**
* A value is pseudolocalizable if it does not define a locale (or is the
* default locale)
- * and is translateable.
+ * and is translatable.
*/
static bool IsPseudolocalizable(ResourceConfigValue* config_value) {
const int diff =
@@ -236,7 +293,7 @@
if (diff & ConfigDescription::CONFIG_LOCALE) {
return false;
}
- return config_value->value->IsTranslateable();
+ return config_value->value->IsTranslatable();
}
} // namespace
diff --git a/tools/aapt2/compile/PseudolocaleGenerator_test.cpp b/tools/aapt2/compile/PseudolocaleGenerator_test.cpp
index 5a9884d..4db37db 100644
--- a/tools/aapt2/compile/PseudolocaleGenerator_test.cpp
+++ b/tools/aapt2/compile/PseudolocaleGenerator_test.cpp
@@ -89,7 +89,7 @@
String* val = test::GetValue<String>(table.get(), "android:string/four");
ASSERT_NE(nullptr, val);
- val->SetTranslateable(false);
+ val->SetTranslatable(false);
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
PseudolocaleGenerator generator;
@@ -130,4 +130,47 @@
test::ParseConfigOrDie("ar-rXB")));
}
+TEST(PseudolocaleGeneratorTest, RespectUntranslateableSections) {
+ std::unique_ptr<IAaptContext> context =
+ test::ContextBuilder().SetCompilationPackage("android").Build();
+ std::unique_ptr<ResourceTable> table = util::make_unique<ResourceTable>();
+
+ {
+ StyleString original_style;
+ original_style.str = "Hello world!";
+ original_style.spans = {Span{"b", 2, 3}, Span{"b", 6, 7}, Span{"i", 1, 10}};
+
+ auto styled_string =
+ util::make_unique<StyledString>(table->string_pool.MakeRef(original_style));
+ styled_string->untranslatable_sections.push_back(UntranslatableSection{6u, 8u});
+ styled_string->untranslatable_sections.push_back(UntranslatableSection{8u, 11u});
+
+ auto string = util::make_unique<String>(table->string_pool.MakeRef(original_style.str));
+ string->untranslatable_sections.push_back(UntranslatableSection{6u, 11u});
+
+ ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("android:string/foo"), ConfigDescription{},
+ {} /* product */, std::move(styled_string),
+ context->GetDiagnostics()));
+ ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("android:string/bar"), ConfigDescription{},
+ {} /* product */, std::move(string), context->GetDiagnostics()));
+ }
+
+ PseudolocaleGenerator generator;
+ ASSERT_TRUE(generator.Consume(context.get(), table.get()));
+
+ StyledString* new_styled_string = test::GetValueForConfig<StyledString>(
+ table.get(), "android:string/foo", test::ParseConfigOrDie("en-rXA"));
+ ASSERT_NE(nullptr, new_styled_string);
+
+ // "world" should be untranslated.
+ EXPECT_NE(std::string::npos, new_styled_string->value->str->find("world"));
+
+ String* new_string = test::GetValueForConfig<String>(table.get(), "android:string/bar",
+ test::ParseConfigOrDie("en-rXA"));
+ ASSERT_NE(nullptr, new_string);
+
+ // "world" should be untranslated.
+ EXPECT_NE(std::string::npos, new_string->value->find("world"));
+}
+
} // namespace aapt
diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index 44d22c4..8bc4e8c 100644
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -1,5 +1,9 @@
# Android Asset Packaging Tool 2.0 (AAPT2) release notes
+## Version 2.7
+### `aapt2 compile`
+- Fixes bug where psuedolocalization auto-translated strings marked 'translateable="false"'.
+
## Version 2.6
### `aapt2`
- Support legacy `configVarying` resource type.