AAPT2: Fix small issue with detecting translatable resources
Change-Id: Idd21b5de4d20be06c6f8c8eb5a22ccd68afc4927
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 2d6c0c2..39a4116 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -446,11 +446,11 @@
}
}
- bool untranslateable = false;
- if (Maybe<StringPiece16> untranslateableAttr = findAttribute(parser, u"untranslateable")) {
- if (!ResourceUtils::tryParseBool(untranslateableAttr.value(), &untranslateable)) {
+ bool translateable = mOptions.translatable;
+ if (Maybe<StringPiece16> translateableAttr = findAttribute(parser, u"translatable")) {
+ if (!ResourceUtils::tryParseBool(translateableAttr.value(), &translateable)) {
mDiag->error(DiagMessage(source)
- << "invalid value for 'untranslateable'. Must be a boolean");
+ << "invalid value for 'translatable'. Must be a boolean");
return false;
}
}
@@ -461,10 +461,10 @@
return false;
}
- if (formatted || untranslateable) {
+ if (formatted && translateable) {
if (String* stringValue = valueCast<String>(outResource->value.get())) {
if (!util::verifyJavaStringFormat(*stringValue->value)) {
- mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber()))
+ mDiag->error(DiagMessage(source)
<< "multiple substitutions specified in non-positional format; "
"did you mean to add the formatted=\"false\" attribute?");
return false;
@@ -973,6 +973,9 @@
const Source source = mSource.withLine(parser->getLineNumber());
std::unique_ptr<Styleable> styleable = util::make_unique<Styleable>();
+ // Declare-styleable is always public, because it technically only exists in R.java.
+ outResource->symbolState = SymbolState::kPublic;
+
std::u16string comment;
bool error = false;
const size_t depth = parser->getDepth();
diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h
index 2d5a29d..06b2581 100644
--- a/tools/aapt2/ResourceParser.h
+++ b/tools/aapt2/ResourceParser.h
@@ -40,6 +40,11 @@
* that don't match before we compile them.
*/
Maybe<std::u16string> product;
+
+ /**
+ * Whether the default setting for this parser is to allow translation.
+ */
+ bool translatable = true;
};
/*
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index 2f5daae..ab7b172 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -52,8 +52,10 @@
Maybe<std::u16string> product = {}) {
std::stringstream input(kXmlPreamble);
input << "<resources>\n" << str << "\n</resources>" << std::endl;
+ ResourceParserOptions parserOptions;
+ parserOptions.product = product;
ResourceParser parser(mContext->getDiagnostics(), &mTable, Source{ "test" }, {},
- ResourceParserOptions{ product });
+ parserOptions);
XmlPullParser xmlParser(input);
if (parser.parse(&xmlParser)) {
return ::testing::AssertionSuccess();
@@ -80,6 +82,14 @@
EXPECT_EQ(std::u16string(u"?123"), *str->value);
}
+TEST_F(ResourceParserTest, ParseFormattedString) {
+ std::string input = "<string name=\"foo\">%d %s</string>";
+ ASSERT_FALSE(testParse(input));
+
+ input = "<string name=\"foo\">%1$d %2$s</string>";
+ ASSERT_TRUE(testParse(input));
+}
+
TEST_F(ResourceParserTest, IgnoreXliffTags) {
std::string input = "<string name=\"foo\" \n"
" xmlns:xliff=\"urn:oasis:names:tc:xliff:document:1.2\">\n"
@@ -322,6 +332,11 @@
"</declare-styleable>";
ASSERT_TRUE(testParse(input));
+ Maybe<ResourceTable::SearchResult> result =
+ mTable.findResource(test::parseNameOrDie(u"@styleable/foo"));
+ AAPT_ASSERT_TRUE(result);
+ EXPECT_EQ(SymbolState::kPublic, result.value().entry->symbolStatus.state);
+
Attribute* attr = test::getValue<Attribute>(&mTable, u"@attr/bar");
ASSERT_NE(attr, nullptr);
EXPECT_TRUE(attr->isWeak());
diff --git a/tools/aapt2/compile/Compile.cpp b/tools/aapt2/compile/Compile.cpp
index 0bc5dce..5d90ab9 100644
--- a/tools/aapt2/compile/Compile.cpp
+++ b/tools/aapt2/compile/Compile.cpp
@@ -132,8 +132,15 @@
// Parse the values file from XML.
XmlPullParser xmlParser(fin);
+
+ ResourceParserOptions parserOptions;
+ parserOptions.product = options.product;
+
+ // If the filename includes donottranslate, then the default translatable is false.
+ parserOptions.translatable = pathData.name.find(u"donottranslate") == std::string::npos;
+
ResourceParser resParser(context->getDiagnostics(), &table, pathData.source,
- pathData.config, ResourceParserOptions{ options.product });
+ pathData.config, parserOptions);
if (!resParser.parse(&xmlParser)) {
return false;
}
diff --git a/tools/aapt2/link/Linkers.h b/tools/aapt2/link/Linkers.h
index 2cc8d9f..7b3fc35 100644
--- a/tools/aapt2/link/Linkers.h
+++ b/tools/aapt2/link/Linkers.h
@@ -38,26 +38,61 @@
bool consume(IAaptContext* context, ResourceTable* table) override;
};
-struct PrivateAttributeMover : public IResourceTableConsumer {
- bool consume(IAaptContext* context, ResourceTable* table) override;
-};
-
struct XmlAutoVersioner : public IXmlResourceConsumer {
bool consume(IAaptContext* context, XmlResource* resource) override;
};
+/**
+ * If any attribute resource values are defined as public, this consumer will move all private
+ * attribute resource values to a private ^private-attr type, avoiding backwards compatibility
+ * issues with new apps running on old platforms.
+ *
+ * The Android platform ignores resource attributes it doesn't recognize, so an app developer can
+ * use new attributes in their layout XML files without worrying about versioning. This assumption
+ * actually breaks on older platforms. OEMs may add private attributes that are used internally.
+ * AAPT originally assigned all private attributes IDs immediately proceeding the public attributes'
+ * IDs.
+ *
+ * This means that on a newer Android platform, an ID previously assigned to a private attribute
+ * may end up assigned to a public attribute.
+ *
+ * App developers assume using the newer attribute is safe on older platforms because it will
+ * be ignored. Instead, the platform thinks the new attribute is an older, private attribute and
+ * will interpret it as such. This leads to unintended styling and exceptions thrown due to
+ * unexpected types.
+ *
+ * By moving the private attributes to a completely different type, this ID conflict will never
+ * occur.
+ */
+struct PrivateAttributeMover : public IResourceTableConsumer {
+ bool consume(IAaptContext* context, ResourceTable* table) override;
+};
+
+/**
+ * Resolves all references to resources in the ResourceTable and assigns them IDs.
+ * The ResourceTable must already have IDs assigned to each resource.
+ * Once the ResourceTable is processed by this linker, it is ready to be flattened.
+ */
struct ReferenceLinker : public IResourceTableConsumer {
bool consume(IAaptContext* context, ResourceTable* table) override;
};
-class XmlReferenceLinker : IXmlResourceConsumer {
+/**
+ * Resolves attributes in the XmlResource and compiles string values to resource values.
+ * Once an XmlResource is processed by this linker, it is ready to be flattened.
+ */
+class XmlReferenceLinker : public IXmlResourceConsumer {
private:
std::set<int> mSdkLevelsFound;
public:
bool consume(IAaptContext* context, XmlResource* resource) override;
- const std::set<int>& getSdkLevels() const {
+ /**
+ * Once the XmlResource has been consumed, this returns the various SDK levels in which
+ * framework attributes used within the XML document were defined.
+ */
+ inline const std::set<int>& getSdkLevels() const {
return mSdkLevelsFound;
}
};