Modernize RenderStyleConstants.h - Part 3
Created attachment 341406 [details] Patch
Attachment 341406 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/TextDecorationPainter.cpp:252: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 341409 [details] Patch
Attachment 341409 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/TextDecorationPainter.cpp:252: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 341409 [details] Patch Attachment 341409 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7818329 Number of test failures exceeded the failure limit.
Created attachment 341410 [details] Archive of layout-test-results from ews117 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 341414 [details] Patch
The fact that you can't construct an OptionSet with a enum value that happens to be 0 is a bit annoying. I'm not clear on the reasoning for it.
Attachment 341414 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/TextDecorationPainter.cpp:252: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 341414 [details] Patch Attachment 341414 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7820300 New failing tests: accessibility/mac/css-speech-speak.html fast/css/round-trip-stroke-width-using-computed-style.html
Created attachment 341415 [details] Archive of layout-test-results from ews112 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 341416 [details] Patch
Attachment 341416 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/TextDecorationPainter.cpp:252: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 341416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341416&action=review r=me with comments. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:-1839 > - if (speakAs & SpeakNormal) Since `SpeakNormal` was 0 previously, I think this path was never executed (speakAs & SpeakNormal is always zero). Is the following code correct? > Source/WebCore/css/CSSLineBoxContainValue.h:35 > +enum LineBoxContainFlags { Should we change this to `enum class` too?
Created attachment 341432 [details] Patch
Attachment 341432 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/TextDecorationPainter.cpp:252: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Yusuke Suzuki from comment #14) > Comment on attachment 341416 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=341416&action=review > > r=me with comments. > > > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:-1839 > > - if (speakAs & SpeakNormal) > > Since `SpeakNormal` was 0 previously, I think this path was never executed > (speakAs & SpeakNormal is always zero). Is the following code correct? That code is funny. The added code doesn't actually change behavior, because the old code ended up doing the same thing at the end of the chain of if-statements if the list was empty. Anyway, no reason to have duplicate functionality so I removed that initial if. > > > Source/WebCore/css/CSSLineBoxContainValue.h:35 > > +enum LineBoxContainFlags { > > Should we change this to `enum class` too? Yeah, I'll get there.
Comment on attachment 341432 [details] Patch Clearing flags on attachment: 341432 Committed r232229: <https://trac.webkit.org/changeset/232229>
All reviewed patches have been landed. Closing bug.
<rdar://problem/40588982>