This hack commonly used for IE-only declarations. It floods the console and should be ignored.
Created attachment 194801 [details] Patch
Created attachment 194816 [details] Patch
Comment on attachment 194816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194816&action=review > Source/WebCore/css/CSSParser.cpp:11288 > + // Ignore syntax error. This trick is used for IE7: There should be no "IE7" in CSS parser. You should filter this error later.
Created attachment 194834 [details] Patch
Comment on attachment 194834 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194834&action=review > Source/WebCore/css/CSSParser.cpp:11273 > +static void AppendToken(StringBuilder* builder, const CSSParserString& token) Methods start with lower case in WebKit. > Source/WebCore/css/CSSParser.cpp:11294 > + builder.appendLiteral("Invalid CSS property declaration at: "); I'd pass enum into single syntaxError and would choose title based on the enum value. > Source/WebCore/inspector/InspectorConsoleAgent.cpp:191 > + if (source == CSSMessageSource && message == "Invalid CSS property declaration at: *") I'd simply look for "*", please also add a comment explaining why this happens.
Created attachment 194841 [details] Patch
Comment on attachment 194841 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194841&action=review > Source/WebCore/css/CSSParser.cpp:11289 > + appendToken(&builder, location.token); You don't need appendToken any longer, do you? > Source/WebCore/css/CSSParser.h:80 > + InvalidPropertyDeclaration, PropertyDeclarationError > Source/WebCore/inspector/InspectorConsoleAgent.cpp:192 > + if (source == CSSMessageSource && message == "Invalid CSS property declaration at: *") You should only look for "*" as I suggested.
Created attachment 194851 [details] Patch
Comment on attachment 194851 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194851&action=review > Source/WebCore/inspector/InspectorConsoleAgent.cpp:193 > + int index = message.find(": "); size_t > Source/WebCore/inspector/InspectorConsoleAgent.cpp:194 > + if (index >= 0 && (int)message.length() > index + 1 && message[index + 2] == '*') static_cast<int>(...)
Comment on attachment 194851 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194851&action=review >> Source/WebCore/inspector/InspectorConsoleAgent.cpp:193 >> + int index = message.find(": "); > > size_t WTF::STring::find has int return value and may return -1.
Created attachment 194858 [details] Patch
Comment on attachment 194858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194858&action=review > Source/WebCore/inspector/InspectorConsoleAgent.cpp:194 > + if (index >= 0 && static_cast<int>(message.length()) > index + 1 && message[index + 2] == '*') Even when message.length() > index + 1, it does not mean message[index + 2] is within range. Isn't it all just find(": *")?
Created attachment 194866 [details] Patch
Comment on attachment 194866 [details] Patch Clearing flags on attachment: 194866 Committed r146781: <http://trac.webkit.org/changeset/146781>
All reviewed patches have been landed. Closing bug.
How about adding an option to enable all logging? We should encourage removing hacks, too.
(In reply to comment #16) > How about adding an option to enable all logging? > We should encourage removing hacks, too. Sure. This is a short term fix. The real one is to make it logged with verbose severity + filter it out on backend by default. Stay tuned.
(In reply to comment #17) > (In reply to comment #16) > > How about adding an option to enable all logging? > > We should encourage removing hacks, too. > > Sure. This is a short term fix. The real one is to make it logged with verbose severity + filter it out on backend by default. Stay tuned. I say "Yay!". :)