Bug 39571

Created attachment 56828 [details] Work in progress

Created attachment 56836 [details] more work in progress

Created attachment 56923 [details] Patch

Attachment 56923 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/html/HTML5Lexer.cpp:890: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/html/HTML5Lexer.cpp:1072: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/html/HTML5Lexer.cpp:1085: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 3 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 56923 [details] Patch WebCore/html/HTML5Lexer.cpp:891 + source.advanceAndASSERTIgnoringCase('d'); Seems it would be simple to write a little helper method (static inline) which did: static inline void advanceAndASSERTIgnoringCase(SegmentedString& string, char* assertionString) { for (int x = 0;; x++) { if (!assertionString[x]) return; string.advanceAndASSERTIngoringCase(assertionString[x]); } } That would read much cleaner in the callsites. WebCore/html/HTML5TreeBuilder.cpp:100 + if (token.type() == HTML5Token::Uninitialized) This seems wrong. WebCore/platform/text/SegmentedString.h:154 + void advanceAndASSERTIgnoringCase(UChar expectedCharacter) We may just want to turn this into a static inline in HTML5Lexer.cpp instead. I think we need one more round.

Created attachment 56932 [details] Patch

Attachment 56932 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/html/HTML5Lexer.cpp:900: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/html/HTML5Lexer.cpp:1077: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/html/HTML5Lexer.cpp:1085: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 3 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 56932 [details] Patch Would be nice if we had a version of this which took a const String& so we didn't have to write "public" twice: 1078 advanceStringAndASSERTIgnoringCase(source, "public"); othewrise looks good. Seems silly to intentionally violate style just to match the spec wording.

> Would be nice if we had a version of this which took a const String& so we didn't have to write "public" twice: This might be a premature optimization, but I'm worried about all the string null checks... > Seems silly to intentionally violate style just to match the spec wording. My hope is that this problem will be sovled once we convert to using goto.

Committed r60087