Attachment 297824[details] did not pass style-queue:
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:84: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 5 in 70 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 297827[details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 297828[details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 297829[details]
Archive of layout-test-results from ews117 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Attachment 297830[details] did not pass style-queue:
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:84: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 5 in 72 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 297849[details] did not pass style-queue:
ERROR: Source/WebCore/dom/MouseEvent.cpp:84: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MutationEvent.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:49: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:50: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
Total errors found: 7 in 81 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 297850[details]
Archive of layout-test-results from ews102 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 297851[details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 297852[details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 297853[details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 297855[details]
Archive of layout-test-results from ews117 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Attachment 297866[details] did not pass style-queue:
ERROR: Source/WebCore/dom/MouseEvent.cpp:84: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MutationEvent.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:49: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:50: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
Total errors found: 7 in 84 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 297866[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=297866&action=review> Source/WebCore/ChangeLog:458
> + * rendering/HitTestResult.cpp:
This ChangeLog entry seems out of place.
> Source/WebCore/dom/DOMNamedFlowCollection.cpp:-67
> - // FIXME: Should be implemented.
You implemented this. Can it be tested?
> Source/WebCore/dom/DOMNamedFlowCollection.idl:39
> - getter WebKitNamedFlow item(unsigned long index);
> - getter WebKitNamedFlow namedItem(DOMString name);
> + getter WebKitNamedFlow? item(unsigned long index);
> + getter WebKitNamedFlow? namedItem(DOMString name);
Can this change be tested?
> Source/WebCore/dom/DocumentMarker.h:99
> + : MarkerTypes(0
This 0 doesn't seem useful.
> Source/WebCore/editing/AlternativeTextController.cpp:731
> Vector<RenderedDocumentMarker*> dictationAlternativesMarkers = markers.markersInRange(selection.get(), DocumentMarker::DictationAlternatives);
auto
Also, could we make markersInRange and markersFor return Vector<std::reference_wrapper<RenderedDocumentMarker>> to make it clear that they can't be null? If we only iterate them and count them, we might be able to get away with using clever iterator types and never allocating a Vector.
Comment on attachment 297866[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=297866&action=review>> Source/WebCore/ChangeLog:458
>> + * rendering/HitTestResult.cpp:
>
> This ChangeLog entry seems out of place.
Oops! Will fix this.
>> Source/WebCore/dom/DOMNamedFlowCollection.cpp:-67
>> - // FIXME: Should be implemented.
>
> You implemented this. Can it be tested?
Good point. I should definitely add a test even though I don’t already know how. I will figure it out.
>> Source/WebCore/dom/DOMNamedFlowCollection.idl:39
>> + getter WebKitNamedFlow? namedItem(DOMString name);
>
> Can this change be tested?
No, there is no change in behavior here. When we specify a return value that can’t be null in an IDL file, our generated code can handle null just fine, and vice versa, so we can get this wrong in IDL files with no repercussions.
>> Source/WebCore/dom/DocumentMarker.h:99
>> + : MarkerTypes(0
>
> This 0 doesn't seem useful.
It is here so that the subsequent lines can all have a "|" at the start of them, rather than having one special one without a "|".
>> Source/WebCore/editing/AlternativeTextController.cpp:731
>> Vector<RenderedDocumentMarker*> dictationAlternativesMarkers = markers.markersInRange(selection.get(), DocumentMarker::DictationAlternatives);
>
> auto
> Also, could we make markersInRange and markersFor return Vector<std::reference_wrapper<RenderedDocumentMarker>> to make it clear that they can't be null? If we only iterate them and count them, we might be able to get away with using clever iterator types and never allocating a Vector.
Sure, we could do those things. I prefer not to do them in this patch, though.
2016-12-28 22:36 PST, Darin Adler
2016-12-29 00:04 PST, Darin Adler
2016-12-29 01:16 PST, Build Bot
2016-12-29 01:22 PST, Build Bot
2016-12-29 01:25 PST, Build Bot
2016-12-29 01:28 PST, Darin Adler
2016-12-30 00:12 PST, Darin Adler
2016-12-30 01:24 PST, Build Bot
2016-12-30 01:28 PST, Build Bot
2016-12-30 01:38 PST, Build Bot
2016-12-30 02:20 PST, Build Bot
2016-12-30 04:19 PST, Build Bot
2016-12-30 19:02 PST, Darin Adler