Add error checking for translatable strings
Translatable strings that have multiple substitutions should use positional String.format() substitutions. This change makes it an error not to use that format on translatable strings that have more than one substitution in its text. Change-Id: I3a19707f3804aa24e8568dc1653a11576cac5916
This commit is contained in:
@ -573,6 +573,7 @@ status_t parseAndAddBag(Bundle* bundle,
|
||||
const String16& parentIdent,
|
||||
const String16& itemIdent,
|
||||
int32_t curFormat,
|
||||
bool isFormatted,
|
||||
bool pseudolocalize,
|
||||
const bool overwrite,
|
||||
ResourceTable* outTable)
|
||||
@ -583,7 +584,7 @@ status_t parseAndAddBag(Bundle* bundle,
|
||||
String16 str;
|
||||
Vector<StringPool::entry_style_span> spans;
|
||||
err = parseStyledString(bundle, in->getPrintableSource().string(),
|
||||
block, item16, &str, &spans,
|
||||
block, item16, &str, &spans, isFormatted,
|
||||
pseudolocalize);
|
||||
if (err != NO_ERROR) {
|
||||
return err;
|
||||
@ -616,6 +617,7 @@ status_t parseAndAddEntry(Bundle* bundle,
|
||||
const String16& curTag,
|
||||
bool curIsStyled,
|
||||
int32_t curFormat,
|
||||
bool isFormatted,
|
||||
bool pseudolocalize,
|
||||
const bool overwrite,
|
||||
ResourceTable* outTable)
|
||||
@ -626,7 +628,7 @@ status_t parseAndAddEntry(Bundle* bundle,
|
||||
Vector<StringPool::entry_style_span> spans;
|
||||
err = parseStyledString(bundle, in->getPrintableSource().string(), block,
|
||||
curTag, &str, curIsStyled ? &spans : NULL,
|
||||
pseudolocalize);
|
||||
isFormatted, pseudolocalize);
|
||||
|
||||
if (err < NO_ERROR) {
|
||||
return err;
|
||||
@ -709,12 +711,18 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
// useful attribute names and special values
|
||||
const String16 name16("name");
|
||||
const String16 translatable16("translatable");
|
||||
const String16 formatted16("formatted");
|
||||
const String16 false16("false");
|
||||
|
||||
const String16 myPackage(assets->getPackage());
|
||||
|
||||
bool hasErrors = false;
|
||||
|
||||
bool fileIsTranslatable = true;
|
||||
if (strstr(in->getPrintableSource().string(), "donottranslate") != NULL) {
|
||||
fileIsTranslatable = false;
|
||||
}
|
||||
|
||||
DefaultKeyedVector<String16, uint32_t> nextPublicId(0);
|
||||
|
||||
ResXMLTree::event_code_t code;
|
||||
@ -751,6 +759,7 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
bool curIsBagReplaceOnOverwrite = false;
|
||||
bool curIsStyled = false;
|
||||
bool curIsPseudolocalizable = false;
|
||||
bool curIsFormatted = fileIsTranslatable;
|
||||
bool localHasErrors = false;
|
||||
|
||||
if (strcmp16(block.getElementName(&len), skip16.string()) == 0) {
|
||||
@ -1136,6 +1145,7 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
String8 locale(rawLocale);
|
||||
String16 name;
|
||||
String16 translatable;
|
||||
String16 formatted;
|
||||
|
||||
size_t n = block.getAttributeCount();
|
||||
for (size_t i = 0; i < n; i++) {
|
||||
@ -1145,11 +1155,14 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
name.setTo(block.getAttributeStringValue(i, &length));
|
||||
} else if (strcmp16(attr, translatable16.string()) == 0) {
|
||||
translatable.setTo(block.getAttributeStringValue(i, &length));
|
||||
} else if (strcmp16(attr, formatted16.string()) == 0) {
|
||||
formatted.setTo(block.getAttributeStringValue(i, &length));
|
||||
}
|
||||
}
|
||||
|
||||
if (name.size() > 0) {
|
||||
if (translatable == false16) {
|
||||
curIsFormatted = false;
|
||||
// Untranslatable strings must only exist in the default [empty] locale
|
||||
if (locale.size() > 0) {
|
||||
fprintf(stderr, "aapt: warning: string '%s' in %s marked untranslatable but exists"
|
||||
@ -1167,6 +1180,10 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
} else {
|
||||
outTable->addLocalization(name, locale);
|
||||
}
|
||||
|
||||
if (formatted == false16) {
|
||||
curIsFormatted = false;
|
||||
}
|
||||
}
|
||||
|
||||
curTag = &string16;
|
||||
@ -1356,7 +1373,7 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
block.getPosition(&parserPosition);
|
||||
|
||||
err = parseAndAddBag(bundle, in, &block, curParams, myPackage, curType,
|
||||
ident, parentIdent, itemIdent, curFormat,
|
||||
ident, parentIdent, itemIdent, curFormat, curIsFormatted,
|
||||
false, overwrite, outTable);
|
||||
if (err == NO_ERROR) {
|
||||
if (curIsPseudolocalizable && localeIsDefined(curParams)
|
||||
@ -1365,8 +1382,8 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
#if 1
|
||||
block.setPosition(parserPosition);
|
||||
err = parseAndAddBag(bundle, in, &block, pseudoParams, myPackage,
|
||||
curType, ident, parentIdent, itemIdent, curFormat, true,
|
||||
overwrite, outTable);
|
||||
curType, ident, parentIdent, itemIdent, curFormat,
|
||||
curIsFormatted, true, overwrite, outTable);
|
||||
#endif
|
||||
}
|
||||
}
|
||||
@ -1389,7 +1406,8 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
block.getPosition(&parserPosition);
|
||||
|
||||
err = parseAndAddEntry(bundle, in, &block, curParams, myPackage, curType, ident,
|
||||
*curTag, curIsStyled, curFormat, false, overwrite, outTable);
|
||||
*curTag, curIsStyled, curFormat, curIsFormatted,
|
||||
false, overwrite, outTable);
|
||||
|
||||
if (err < NO_ERROR) { // Why err < NO_ERROR instead of err != NO_ERROR?
|
||||
hasErrors = localHasErrors = true;
|
||||
@ -1400,7 +1418,8 @@ status_t compileResourceFile(Bundle* bundle,
|
||||
// pseudolocalize here
|
||||
block.setPosition(parserPosition);
|
||||
err = parseAndAddEntry(bundle, in, &block, pseudoParams, myPackage, curType,
|
||||
ident, *curTag, curIsStyled, curFormat, true, overwrite, outTable);
|
||||
ident, *curTag, curIsStyled, curFormat,
|
||||
curIsFormatted, true, overwrite, outTable);
|
||||
if (err != NO_ERROR) {
|
||||
hasErrors = localHasErrors = true;
|
||||
}
|
||||
|
@ -68,12 +68,118 @@ String16 getNamespaceResourcePackage(String16 namespaceUri, bool* outIsPublic)
|
||||
return String16(namespaceUri, namespaceUri.size()-prefixSize, prefixSize);
|
||||
}
|
||||
|
||||
status_t hasSubstitutionErrors(const char* fileName,
|
||||
ResXMLTree* inXml,
|
||||
String16 str16)
|
||||
{
|
||||
const char16_t* str = str16.string();
|
||||
const char16_t* p = str;
|
||||
const char16_t* end = str + str16.size();
|
||||
|
||||
bool nonpositional = false;
|
||||
int argCount = 0;
|
||||
|
||||
while (p < end) {
|
||||
/*
|
||||
* Look for the start of a Java-style substitution sequence.
|
||||
*/
|
||||
if (*p == '%' && p + 1 < end) {
|
||||
p++;
|
||||
|
||||
// A literal percent sign represented by %%
|
||||
if (*p == '%') {
|
||||
p++;
|
||||
continue;
|
||||
}
|
||||
|
||||
argCount++;
|
||||
|
||||
if (*p >= '0' && *p <= '9') {
|
||||
do {
|
||||
p++;
|
||||
} while (*p >= '0' && *p <= '9');
|
||||
if (*p != '$') {
|
||||
// This must be a size specification instead of position.
|
||||
nonpositional = true;
|
||||
}
|
||||
} else if (*p == '<') {
|
||||
// Reusing last argument; bad idea since it can be re-arranged.
|
||||
nonpositional = true;
|
||||
p++;
|
||||
|
||||
// Optionally '$' can be specified at the end.
|
||||
if (p < end && *p == '$') {
|
||||
p++;
|
||||
}
|
||||
} else {
|
||||
nonpositional = true;
|
||||
}
|
||||
|
||||
// Ignore flags and widths
|
||||
while (p < end && (*p == '-' ||
|
||||
*p == '#' ||
|
||||
*p == '+' ||
|
||||
*p == ' ' ||
|
||||
*p == ',' ||
|
||||
*p == '(' ||
|
||||
(*p >= '0' && *p <= '9'))) {
|
||||
p++;
|
||||
}
|
||||
|
||||
/*
|
||||
* This is a shortcut to detect strings that are going to Time.format()
|
||||
* instead of String.format()
|
||||
*
|
||||
* Comparison of String.format() and Time.format() args:
|
||||
*
|
||||
* String: ABC E GH ST X abcdefgh nost x
|
||||
* Time: DEFGHKMS W Za d hkm s w yz
|
||||
*
|
||||
* Therefore we know it's definitely Time if we have:
|
||||
* DFKMWZkmwyz
|
||||
*/
|
||||
if (p < end) {
|
||||
switch (*p) {
|
||||
case 'D':
|
||||
case 'F':
|
||||
case 'K':
|
||||
case 'M':
|
||||
case 'W':
|
||||
case 'Z':
|
||||
case 'k':
|
||||
case 'm':
|
||||
case 'w':
|
||||
case 'y':
|
||||
case 'z':
|
||||
return NO_ERROR;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
p++;
|
||||
}
|
||||
|
||||
/*
|
||||
* If we have more than one substitution in this string and any of them
|
||||
* are not in positional form, give the user an error.
|
||||
*/
|
||||
if (argCount > 1 && nonpositional) {
|
||||
SourcePos(String8(fileName), inXml->getLineNumber()).error(
|
||||
"Multiple substitutions specified in non-positional format; "
|
||||
"did you mean to add the formatted=\"true\" attribute?\n");
|
||||
return NOT_ENOUGH_DATA;
|
||||
}
|
||||
|
||||
return NO_ERROR;
|
||||
}
|
||||
|
||||
status_t parseStyledString(Bundle* bundle,
|
||||
const char* fileName,
|
||||
ResXMLTree* inXml,
|
||||
const String16& endTag,
|
||||
String16* outString,
|
||||
Vector<StringPool::entry_style_span>* outSpans,
|
||||
bool isFormatted,
|
||||
bool pseudolocalize)
|
||||
{
|
||||
Vector<StringPool::entry_style_span> spanStack;
|
||||
@ -100,9 +206,13 @@ status_t parseStyledString(Bundle* bundle,
|
||||
std::string orig(String8(text).string());
|
||||
std::string pseudo = pseudolocalize_string(orig);
|
||||
curString.append(String16(String8(pseudo.c_str())));
|
||||
} else {
|
||||
if (isFormatted && hasSubstitutionErrors(fileName, inXml, text) != NO_ERROR) {
|
||||
return UNKNOWN_ERROR;
|
||||
} else {
|
||||
curString.append(text);
|
||||
}
|
||||
}
|
||||
} else if (code == ResXMLTree::START_TAG) {
|
||||
const String16 element16(inXml->getElementName(&len));
|
||||
const String8 element8(element16);
|
||||
|
@ -25,6 +25,7 @@ status_t parseStyledString(Bundle* bundle,
|
||||
const String16& endTag,
|
||||
String16* outString,
|
||||
Vector<StringPool::entry_style_span>* outSpans,
|
||||
bool isFormatted,
|
||||
bool isPseudolocalizable);
|
||||
|
||||
void printXMLBlock(ResXMLTree* block);
|
||||
|
Reference in New Issue
Block a user