From 35c608da334a0bab64c4c524b6fb9a98be6c371d Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Thu, 15 Nov 2012 11:36:15 -0500 Subject: [PATCH] Bug 455839: simplify the CSS error reporting API. r=dbaron --- layout/style/ErrorReporter.cpp | 100 ++++++++++++++++----------------- layout/style/ErrorReporter.h | 71 ++++++++--------------- layout/style/nsCSSParser.cpp | 75 +++++++------------------ layout/style/nsCSSScanner.cpp | 4 +- 4 files changed, 92 insertions(+), 158 deletions(-) diff --git a/layout/style/ErrorReporter.cpp b/layout/style/ErrorReporter.cpp index 690b453b190..91f73d0e41d 100644 --- a/layout/style/ErrorReporter.cpp +++ b/layout/style/ErrorReporter.cpp @@ -221,7 +221,7 @@ ErrorReporter::ClearError() } void -ErrorReporter::AddToError(const nsAString &aErrorText) +ErrorReporter::AddToError(const nsString &aErrorText) { if (!ShouldReportErrors()) return; @@ -230,7 +230,8 @@ ErrorReporter::AddToError(const nsAString &aErrorText) mErrorColNumber = mScanner->GetColumnNumber(); mError = aErrorText; } else { - mError.Append(NS_LITERAL_STRING(" ") + aErrorText); + mError.AppendLiteral(" "); + mError.Append(aErrorText); } } @@ -246,15 +247,51 @@ ErrorReporter::ReportUnexpected(const char *aMessage) } void -ErrorReporter::ReportUnexpectedParams(const char *aMessage, - const PRUnichar **aParams, - uint32_t aParamsLength) +ErrorReporter::ReportUnexpected(const char *aMessage, + const nsString &aParam) { if (!ShouldReportErrors()) return; + const PRUnichar *params[1] = { aParam.get() }; nsAutoString str; sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(), - aParams, aParamsLength, + params, ArrayLength(params), + getter_Copies(str)); + AddToError(str); +} + +void +ErrorReporter::ReportUnexpected(const char *aMessage, + const nsCSSToken &aToken) +{ + if (!ShouldReportErrors()) return; + + nsAutoString tokenString; + aToken.AppendToString(tokenString); + const PRUnichar *params[1] = { tokenString.get() }; + + nsAutoString str; + sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(), + params, ArrayLength(params), + getter_Copies(str)); + AddToError(str); +} + +void +ErrorReporter::ReportUnexpected(const char *aMessage, + const nsCSSToken &aToken, + PRUnichar aChar) +{ + if (!ShouldReportErrors()) return; + + nsAutoString tokenString; + aToken.AppendToString(tokenString); + const PRUnichar charStr[2] = { aChar, 0 }; + const PRUnichar *params[2] = { tokenString.get(), charStr }; + + nsAutoString str; + sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(), + params, ArrayLength(params), getter_Copies(str)); AddToError(str); } @@ -267,13 +304,11 @@ ErrorReporter::ReportUnexpectedEOF(const char *aMessage) nsAutoString innerStr; sStringBundle->GetStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(), getter_Copies(innerStr)); - const PRUnichar *params[] = { - innerStr.get() - }; + const PRUnichar *params[1] = { innerStr.get() }; nsAutoString str; sStringBundle->FormatStringFromName(NS_LITERAL_STRING("PEUnexpEOF2").get(), - params, NS_ARRAY_LENGTH(params), + params, ArrayLength(params), getter_Copies(str)); AddToError(str); } @@ -286,56 +321,15 @@ ErrorReporter::ReportUnexpectedEOF(PRUnichar aExpected) const PRUnichar expectedStr[] = { PRUnichar('\''), aExpected, PRUnichar('\''), PRUnichar(0) }; - const PRUnichar *params[] = { expectedStr }; + const PRUnichar *params[1] = { expectedStr }; nsAutoString str; sStringBundle->FormatStringFromName(NS_LITERAL_STRING("PEUnexpEOF2").get(), - params, NS_ARRAY_LENGTH(params), + params, ArrayLength(params), getter_Copies(str)); AddToError(str); } -void -ErrorReporter::ReportUnexpectedToken(const char *aMessage, - const nsCSSToken &aToken) -{ - if (!ShouldReportErrors()) return; - - nsAutoString tokenString; - aToken.AppendToString(tokenString); - const PRUnichar *params[] = { - tokenString.get() - }; - - nsAutoString str; - sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(), - params, NS_ARRAY_LENGTH(params), - getter_Copies(str)); - AddToError(str); -} - -void -ErrorReporter::ReportUnexpectedTokenParams(const char *aMessage, - const nsCSSToken &aToken, - const PRUnichar **aParams, - uint32_t aParamsLength) -{ - NS_ABORT_IF_FALSE(aParams[0] == nullptr, "first param should be empty"); - - if (!ShouldReportErrors()) return; - - nsAutoString tokenString; - aToken.AppendToString(tokenString); - aParams[0] = tokenString.get(); - - nsAutoString str; - sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(), - aParams, aParamsLength, - getter_Copies(str)); - AddToError(str); - -} - } // namespace css } // namespace mozilla diff --git a/layout/style/ErrorReporter.h b/layout/style/ErrorReporter.h index 9fe82e46903..f1a68573e77 100644 --- a/layout/style/ErrorReporter.h +++ b/layout/style/ErrorReporter.h @@ -38,47 +38,28 @@ public: void OutputError(); void ClearError(); - // aMessage must take no parameters + // In all overloads of ReportUnexpected, aMessage is a stringbundle + // name, which will be processed as a format string with the + // indicated number of parameters. + + // no parameters void ReportUnexpected(const char *aMessage); - // aLookingFor is a plain string, not a format string - void ReportUnexpectedEOF(const char *aLookingFor); - // aLookingFor is a single character - void ReportUnexpectedEOF(PRUnichar aLookingFor); + // one parameter, a string + void ReportUnexpected(const char *aMessage, const nsString& aParam); + // one parameter, a token + void ReportUnexpected(const char *aMessage, const nsCSSToken& aToken); + // two parameters, a token and a character, in that order + void ReportUnexpected(const char *aMessage, const nsCSSToken& aToken, + PRUnichar aChar); - // aMessage is a format string with 1 parameter (for the string - // representation of the unexpected token) - void ReportUnexpectedToken(const char *aMessage, const nsCSSToken &aToken); - - // aMessage is a format string - template - void ReportUnexpectedParams(const char* aMessage, - const PRUnichar* (&aParams)[N]) - { - MOZ_STATIC_ASSERT(N > 0, "use ReportUnexpected instead"); - ReportUnexpectedParams(aMessage, aParams, N); - } - - // aMessage is a format string - // aParams's first entry must be null, and we'll fill in the token - template - void ReportUnexpectedTokenParams(const char* aMessage, - const nsCSSToken &aToken, - const PRUnichar* (&aParams)[N]) - { - MOZ_STATIC_ASSERT(N > 1, "use ReportUnexpectedToken instead"); - ReportUnexpectedTokenParams(aMessage, aToken, aParams, N); - } + // for ReportUnexpectedEOF, aExpected can be either a stringbundle + // name or a single character. In the former case there may not be + // any format parameters. + void ReportUnexpectedEOF(const char *aExpected); + void ReportUnexpectedEOF(PRUnichar aExpected); private: - void ReportUnexpectedParams(const char *aMessage, - const PRUnichar **aParams, - uint32_t aParamsLength); - void ReportUnexpectedTokenParams(const char *aMessage, - const nsCSSToken &aToken, - const PRUnichar **aParams, - uint32_t aParamsLength); - - void AddToError(const nsAString &aErrorText); + void AddToError(const nsString &aErrorText); #ifdef CSS_REPORT_PARSE_ERRORS nsAutoString mError; @@ -106,19 +87,15 @@ inline void ErrorReporter::OutputError() {} inline void ErrorReporter::ClearError() {} inline void ErrorReporter::ReportUnexpected(const char *) {} -inline void ErrorReporter::ReportUnexpectedParams(const char *, - const PRUnichar **, - uint32_t) {} +inline void ErrorReporter::ReportUnexpected(const char *, const nsString &) {} +inline void ErrorReporter::ReportUnexpected(const char *, const nsCSSToken &) {} +inline void ErrorReporter::ReportUnexpected(const char *, const nsCSSToken &, + PRUnichar) {} + inline void ErrorReporter::ReportUnexpectedEOF(const char *) {} inline void ErrorReporter::ReportUnexpectedEOF(PRUnichar) {} -inline void ErrorReporter::ReportUnexpectedToken(const char *, - const nsCSSToken &) {} -inline void ErrorReporter::ReportUnexpectedTokenParams(const char *, - const nsCSSToken &, - const PRUnichar **, - uint32_t) {} -inline void ErrorReporter::AddToError(const nsAString &) {} +inline void ErrorReporter::AddToError(const nsString &) {} #endif } // namespace css diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index 3e0a956c777..bd1594bf9d6 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -721,8 +721,14 @@ static void AppendRuleToSheet(css::Rule* aRule, void* aParser) #define REPORT_UNEXPECTED(msg_) \ mReporter->ReportUnexpected(#msg_) -#define REPORT_UNEXPECTED_P(msg_, params_) \ - mReporter->ReportUnexpectedParams(#msg_, params_) +#define REPORT_UNEXPECTED_P(msg_, param_) \ + mReporter->ReportUnexpected(#msg_, param_) + +#define REPORT_UNEXPECTED_TOKEN(msg_) \ + mReporter->ReportUnexpected(#msg_, mToken) + +#define REPORT_UNEXPECTED_TOKEN_CHAR(msg_, ch_) \ + mReporter->ReportUnexpected(#msg_, mToken, ch_) #define REPORT_UNEXPECTED_EOF(lf_) \ mReporter->ReportUnexpectedEOF(#lf_) @@ -730,12 +736,6 @@ static void AppendRuleToSheet(css::Rule* aRule, void* aParser) #define REPORT_UNEXPECTED_EOF_CHAR(ch_) \ mReporter->ReportUnexpectedEOF(ch_) -#define REPORT_UNEXPECTED_TOKEN(msg_) \ - mReporter->ReportUnexpectedToken(#msg_, mToken) - -#define REPORT_UNEXPECTED_TOKEN_P(msg_, params_) \ - mReporter->ReportUnexpectedTokenParams(#msg_, mToken, params_) - #define OUTPUT_ERROR() \ mReporter->OutputError() @@ -1088,10 +1088,7 @@ CSSParserImpl::ParseProperty(const nsCSSProperty aPropID, // Check for unknown or preffed off properties if (eCSSProperty_UNKNOWN == aPropID || !nsCSSProps::IsEnabled(aPropID)) { NS_ConvertASCIItoUTF16 propName(nsCSSProps::GetStringValue(aPropID)); - const PRUnichar *params[] = { - propName.get() - }; - REPORT_UNEXPECTED_P(PEUnknownProperty, params); + REPORT_UNEXPECTED_P(PEUnknownProperty, propName); REPORT_UNEXPECTED(PEDeclDropped); OUTPUT_ERROR(); ReleaseScanner(); @@ -1107,10 +1104,7 @@ CSSParserImpl::ParseProperty(const nsCSSProperty aPropID, if (!parsedOK) { NS_ConvertASCIItoUTF16 propName(nsCSSProps::GetStringValue(aPropID)); - const PRUnichar *params[] = { - propName.get() - }; - REPORT_UNEXPECTED_P(PEValueParsingError, params); + REPORT_UNEXPECTED_P(PEValueParsingError, propName); REPORT_UNEXPECTED(PEDeclDropped); OUTPUT_ERROR(); mTempData.ClearProperty(aPropID); @@ -1929,10 +1923,7 @@ CSSParserImpl::ProcessImport(const nsString& aURLSpec, if (NS_FAILED(rv)) { if (rv == NS_ERROR_MALFORMED_URI) { // import url is bad - const PRUnichar *params[] = { - aURLSpec.get() - }; - REPORT_UNEXPECTED_P(PEImportBadURI, params); + REPORT_UNEXPECTED_P(PEImportBadURI, aURLSpec); OUTPUT_ERROR(); } return; @@ -2213,19 +2204,13 @@ CSSParserImpl::ParseFontDescriptor(nsCSSFontFaceRule* aRule) SkipDeclaration(true); return true; } else { - const PRUnichar *params[] = { - descName.get() - }; - REPORT_UNEXPECTED_P(PEUnknownFontDesc, params); + REPORT_UNEXPECTED_P(PEUnknownFontDesc, descName); return false; } } if (!ParseFontDescriptorValue(descID, value)) { - const PRUnichar *params[] = { - descName.get() - }; - REPORT_UNEXPECTED_P(PEValueParsingError, params); + REPORT_UNEXPECTED_P(PEValueParsingError, descName); return false; } @@ -2491,10 +2476,7 @@ CSSParserImpl::ParseSupportsConditionInParensInsideParens(bool& aConditionMet) } if (ExpectSymbol(')', true)) { - const PRUnichar *params[] = { - propertyName.get() - }; - REPORT_UNEXPECTED_P(PEValueParsingError, params); + REPORT_UNEXPECTED_P(PEValueParsingError, propertyName); UngetToken(); return false; } @@ -4134,12 +4116,7 @@ CSSParserImpl::ParseColorComponent(uint8_t& aComponent, aComponent = NSToIntRound(value); return true; } - const PRUnichar stopString[] = { PRUnichar(aStop), PRUnichar(0) }; - const PRUnichar *params[] = { - nullptr, - stopString - }; - REPORT_UNEXPECTED_TOKEN_P(PEColorComponentBadTerm, params); + REPORT_UNEXPECTED_TOKEN_CHAR(PEColorComponentBadTerm, aStop); return false; } @@ -4208,12 +4185,7 @@ CSSParserImpl::ParseHSLColor(nscolor& aColor, return true; } - const PRUnichar stopString[] = { PRUnichar(aStop), PRUnichar(0) }; - const PRUnichar *params[] = { - nullptr, - stopString - }; - REPORT_UNEXPECTED_TOKEN_P(PEColorComponentBadTerm, params); + REPORT_UNEXPECTED_TOKEN_CHAR(PEColorComponentBadTerm, aStop); return false; } @@ -4341,10 +4313,7 @@ CSSParserImpl::ParseDeclaration(css::Declaration* aDeclaration, (aContext == nsCSSContextType::eCSSContext_Page && !nsCSSProps::PropHasFlags(propID, CSS_PROPERTY_APPLIES_TO_PAGE_RULE))) { // unknown property if (!NonMozillaVendorIdentifier(propertyName)) { - const PRUnichar *params[] = { - propertyName.get() - }; - REPORT_UNEXPECTED_P(PEUnknownProperty, params); + REPORT_UNEXPECTED_P(PEUnknownProperty, propertyName); REPORT_UNEXPECTED(PEDeclDropped); OUTPUT_ERROR(); } @@ -4353,10 +4322,7 @@ CSSParserImpl::ParseDeclaration(css::Declaration* aDeclaration, } if (! ParseProperty(propID)) { // XXX Much better to put stuff in the value parsers instead... - const PRUnichar *params[] = { - propertyName.get() - }; - REPORT_UNEXPECTED_P(PEValueParsingError, params); + REPORT_UNEXPECTED_P(PEValueParsingError, propertyName); REPORT_UNEXPECTED(PEDeclDropped); OUTPUT_ERROR(); mTempData.ClearProperty(propID); @@ -9861,10 +9827,7 @@ CSSParserImpl::GetNamespaceIdForPrefix(const nsString& aPrefix) // else no declared namespaces if (nameSpaceID == kNameSpaceID_Unknown) { // unknown prefix, dump it - const PRUnichar *params[] = { - aPrefix.get() - }; - REPORT_UNEXPECTED_P(PEUnknownNamespacePrefix, params); + REPORT_UNEXPECTED_P(PEUnknownNamespacePrefix, aPrefix); } return nameSpaceID; diff --git a/layout/style/nsCSSScanner.cpp b/layout/style/nsCSSScanner.cpp index 416075557ed..e3e36faf1a5 100644 --- a/layout/style/nsCSSScanner.cpp +++ b/layout/style/nsCSSScanner.cpp @@ -990,7 +990,7 @@ nsCSSScanner::ParseString(int32_t aStop, nsCSSToken& aToken) } if (ch == '\n') { aToken.mType = eCSSToken_Bad_String; - mReporter->ReportUnexpectedToken("SEUnterminatedString", aToken); + mReporter->ReportUnexpected("SEUnterminatedString", aToken); break; } if (ch == '\\') { @@ -1004,7 +1004,7 @@ nsCSSScanner::ParseString(int32_t aStop, nsCSSToken& aToken) // the backslash, but as far as the author is concerned, it // works pretty much the same as an unterminated string, so we // use the same error message. - mReporter->ReportUnexpectedToken("SEUnterminatedString", aToken); + mReporter->ReportUnexpected("SEUnterminatedString", aToken); break; } } else {