Bug 161537

Created attachment 287780 [details] Patch

Attachment 287780 [details] did not pass style-queue: ERROR: Source/WebCore/css/parser/MediaQueryParser.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/MediaQueryParser.h:93: 'parseImpl' is incorrectly named. It should be named 'protector' or 'protectedCSSParserTokenRange'. [readability/naming/protected] [4] ERROR: Source/WebCore/css/parser/MediaQueryParser.cpp:131: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/MediaQueryParser.cpp:156: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/MediaQueryParser.cpp:158: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/MediaQueryParser.cpp:176: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/MediaQueryParser.cpp:193: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/MediaQueryParser.cpp:213: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/MediaQueryExp.cpp:68: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/css/MediaQueryExp.cpp:291: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/css/MediaQueryExp.cpp:311: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/css/MediaQueryExp.cpp:312: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 12 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 287786 [details] Patch

Attachment 287786 [details] did not pass style-queue: ERROR: Source/WebCore/css/parser/MediaQueryParser.h:93: 'parseImpl' is incorrectly named. It should be named 'protector' or 'protectedCSSParserTokenRange'. [readability/naming/protected] [4] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 287793 [details] Patch

Attachment 287793 [details] did not pass style-queue: ERROR: Source/WebCore/css/parser/MediaQueryParser.h:93: 'parseInternal' is incorrectly named. It should be named 'protector' or 'protectedCSSParserTokenRange'. [readability/naming/protected] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 287793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287793&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:3355 > - 946D37401D6CE3C20077084F /* CSSParserToken.h in Headers */ = {isa = PBXBuildFile; fileRef = 946D373D1D6CE31A0077084F /* CSSParserToken.h */; }; > + 946D37401D6CE3C20077084F /* CSSParserToken.h in Headers */ = {isa = PBXBuildFile; fileRef = 946D373D1D6CE31A0077084F /* CSSParserToken.h */; settings = {ATTRIBUTES = (Private, ); }; }; This looks unintentional. > Source/WebCore/css/MediaQueryExp.cpp:71 > + > + Nit: extra blank line > Source/WebCore/css/MediaQueryExp.cpp:321 > + // TODO(timloh): <ratio> is supposed to allow whitespace around the '/' Did you want to leave this in? > Source/WebCore/css/MediaQueryExp.h:37 > - > + oops > Source/WebCore/css/parser/CSSParserIdioms.cpp:39 > +template<typename CharacterType> ALWAYS_INLINE static void convertToASCIILowercaseInPlace(CharacterType* characters, unsigned length) > +{ > + for (unsigned i = 0; i < length; ++i) > + characters[i] = toASCIILower(characters[i]); > +} You have this code in CSSParserToken.cpp too. > Source/WebCore/css/parser/MediaQueryBlockWatcher.cpp:39 > +MediaQueryBlockWatcher::MediaQueryBlockWatcher() > +{ > +} Can you just put this in the header? Or leave it out? > Source/WebCore/css/parser/MediaQueryBlockWatcher.cpp:45 > + if (token.getBlockType() == CSSParserToken::BlockStart) { > + ++m_blockLevel; > + } else if (token.getBlockType() == CSSParserToken::BlockEnd) { Nit: Single line conditional. > Source/WebCore/css/parser/MediaQueryParser.cpp:85 > +// State machine member functions start here Remove? > Source/WebCore/css/parser/MediaQueryParser.cpp:124 > + } else if (m_state == ReadRestrictor && equalIgnoringASCIICase(token.value(), "only")) { > + setStateAndRestrict(ReadMediaType, MediaQuery::Only); > + } else if (m_mediaQueryData.restrictor() != MediaQuery::None > + && isRestrictorOrLogicalOperator(token)) { > + m_state = SkipUntilComma; > + } else { > + StringView stringView = token.value(); Single line conditions here. > Source/WebCore/css/parser/MediaQueryParser.cpp:151 > + if (type == IdentToken && equalIgnoringASCIICase(token.value(), "and")) { > + m_state = ReadFeatureStart; > + } else if (type == CommaToken && m_parserType != MediaConditionParser) { ANd here. > Source/WebCore/css/parser/MediaQueryParser.h:55 > +private: > + MediaQuery::Restrictor m_restrictor; > + String m_mediaType; > + Vector<MediaQueryExpression> m_expressions; > + String m_mediaFeature; > + Vector<CSSParserToken, 4> m_valueList; > + bool m_mediaTypeSet; > + > +public: Why is private before public here?

Comment on attachment 287793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287793&action=review > Source/WebCore/css/MediaQueryExp.cpp:71 > + Extra newline. > Source/WebCore/css/parser/MediaQueryBlockWatcher.h:31 > +#ifndef MediaQueryBlockWatcher_h > +#define MediaQueryBlockWatcher_h We are doing #pragma once now, so you can replace this and the #endif with it. > Source/WebCore/css/parser/MediaQueryParser.h:31 > +#ifndef MediaQueryParser_h > +#define MediaQueryParser_h We are doing #pragma once now, so you can replace this and the #endif with it. > Source/WebCore/css/parser/MediaQueryParser.h:136 > + > +}; Extra newline.

Landed in r205368.