OSDN Git Service

AAPT2: Fix pseudolocalization to respect <xliff:g>
authorAdam Lesinski <adamlesinski@google.com>
Fri, 6 Jan 2017 23:20:04 +0000 (15:20 -0800)
committerAdam Lesinski <adamlesinski@google.com>
Wed, 8 Feb 2017 14:23:45 +0000 (06:23 -0800)
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

12 files changed:
libs/androidfw/include/androidfw/StringPiece.h
tools/aapt2/Main.cpp
tools/aapt2/ResourceParser.cpp
tools/aapt2/ResourceParser.h
tools/aapt2/ResourceParser_test.cpp
tools/aapt2/ResourceValues.cpp
tools/aapt2/ResourceValues.h
tools/aapt2/StringPool.cpp
tools/aapt2/StringPool.h
tools/aapt2/compile/PseudolocaleGenerator.cpp
tools/aapt2/compile/PseudolocaleGenerator_test.cpp
tools/aapt2/readme.md

index c9effd1..02cc505 100644 (file)
@@ -261,8 +261,36 @@ inline ::std::ostream& operator<<(::std::ostream& out, const BasicStringPiece<ch
   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>
index 227ffa3..ba378d7 100644 (file)
@@ -25,7 +25,7 @@ namespace aapt {
 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 << "."
index 1c750c6..47ca266 100644 (file)
@@ -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 @@ ResourceParser::ResourceParser(IDiagnostics* diag, ResourceTable* table,
 /**
  * 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;
-      }
-
-      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();
+    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;
+        }
 
-    } else if (event == xml::XmlPullParser::Event::kText) {
-      out_raw_string->append(parser->text());
-      builder.Append(parser->text());
+        // 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;
+        }
 
-    } 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() << "'");
+        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});
+          }
         }
-        continue;
-      }
-      depth++;
+        // Ignore other xliff tags, they get handled by other tools.
 
-      // 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;
+      } 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() << "'");
       }
 
-      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())});
+      // 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;
       }
 
+      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 = {};
+      }
     } 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::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser,
 
   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 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser,
   // 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 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser,
     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 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser,
   }
 
   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 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser,
       }
     }
 
-  } 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 @@ bool ResourceParser::ParseArrayImpl(xml::XmlPullParser* parser,
 
   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();
index cc0fa26..8258019 100644 (file)
@@ -60,16 +60,16 @@ class ResourceParser {
  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.
index cf901da..67ed476 100644 (file)
@@ -76,6 +76,7 @@ TEST_F(ResourceParserTest, ParseQuotedString) {
   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 @@ TEST_F(ResourceParserTest, ParseEscapedString) {
   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, ParseFormattedString) {
 
 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 @@ TEST_F(ResourceParserTest, ParseStyledString) {
   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 @@ TEST_F(ResourceParserTest, ParseStringWithWhitespace) {
   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 @@ TEST_F(ResourceParserTest, ParseStringWithWhitespace) {
   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 @@ TEST_F(ResourceParserTest, ParseNull) {
 
   // 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);
 }
 
index 7956ad8..f75ed7a 100644 (file)
@@ -140,7 +140,23 @@ bool String::Equals(const Value* value) const {
   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* String::Clone(StringPool* new_pool) const {
   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 @@ bool StyledString::Equals(const Value* value) const {
     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;
+  }
+
+  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 false;
+  return true;
 }
 
 bool StyledString::Flatten(android::Res_value* out_value) const {
@@ -200,6 +222,7 @@ StyledString* StyledString::Clone(StringPool* new_pool) const {
   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 @@ bool FileReference::Equals(const Value* value) const {
   if (!other) {
     return false;
   }
-  return *path == *other->path;
+  return path == other->path;
 }
 
 bool FileReference::Flatten(android::Res_value* out_value) const {
index d380f8d..c71c738 100644 (file)
@@ -34,44 +34,35 @@ namespace aapt {
 
 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 @@ struct Value {
 
   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 @@ struct Reference : public BaseItem<Reference> {
 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 @@ struct Id : public BaseItem<Id> {
   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 @@ struct RawString : public BaseItem<RawString> {
   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 String : public BaseItem<String> {
 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 StyledString : public BaseItem<StyledString> {
 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 @@ struct FileReference : public BaseItem<FileReference> {
   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 @@ struct Style : public BaseValue<Style> {
 
   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 @@ struct Styleable : public BaseValue<Styleable> {
   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;
index d968d73..57da5f0 100644 (file)
@@ -63,6 +63,14 @@ StringPool::Ref& StringPool::Ref::operator=(const StringPool::Ref& rhs) {
   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 @@ StringPool::StyleRef& StringPool::StyleRef::operator=(
   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_;
 }
index d0ce489..a626d37 100644 (file)
@@ -70,6 +70,8 @@ class StringPool {
     ~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 @@ class StringPool {
     ~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;
 
index 5035f81..fad9edd 100644 (file)
@@ -43,16 +43,16 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString(
   }
 
   // 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 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString(
 
   // 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 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString(
     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 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString(
     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 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString(
       *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 @@ class Visitor : public RawValueVisitor {
   }
 
   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 @@ class Visitor : public RawValueVisitor {
 
   void Visit(StyledString* string) override {
     item = PseudolocalizeStyledString(string, method_, pool_);
+    item->SetSource(string->GetSource());
     item->SetWeak(true);
   }
 
@@ -228,7 +285,7 @@ void PseudolocalizeIfNeeded(const Pseudolocalizer::Method method,
 /**
  * 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 @@ static bool IsPseudolocalizable(ResourceConfigValue* config_value) {
   if (diff & ConfigDescription::CONFIG_LOCALE) {
     return false;
   }
-  return config_value->value->IsTranslateable();
+  return config_value->value->IsTranslatable();
 }
 
 }  // namespace
index 5a9884d..4db37db 100644 (file)
@@ -89,7 +89,7 @@ TEST(PseudolocaleGeneratorTest, PseudolocalizeOnlyDefaultConfigs) {
 
   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(PseudolocaleGeneratorTest, PseudolocalizeOnlyDefaultConfigs) {
                                             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
index 44d22c4..8bc4e8c 100644 (file)
@@ -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.