No ax tests are running in the simulator because they were disabled and support not added <rdar://problem/21390968>
Created attachment 254931 [details] patch
Attachment 254931 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:38: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:1003: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:1007: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:1011: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:1015: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:1019: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 7 in 70 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 254936 [details] patch
Comment on attachment 254936 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=254936&action=review Looks sane to me. > Tools/ChangeLog:11 > + rename to more generic names. Minor: Please remove the indentation on this line such that the 'r' in rename left aligns with the 'R' in rename of the previous line. > Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:715 > + // Mac programmers should not be adding more than one notification listener per element. Did you mean to write "Mac programmers" instead of "iOS programmers" or "Mac and iOS programmers"? I mean this comment is in an iOS-specific file. I take it you wrote the comment using "Mac programmers" because iOS makes use of the Mac accessibility code. > Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:729 > + // Mac programmers should not be trying to remove a listener that's already removed. Ditto. > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:142 > + return 0; 0 => nullptr > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:151 > + return 0; Ditto. > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:195 > + JSValueRef arrayResult = JSObjectMakeArray(context, 0, 0, 0); We should pass nullptr for the last two arguments instead of 0. > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:196 > + JSObjectRef arrayObj = JSValueToObject(context, arrayResult, 0); We should pass nullptr instead of 0 to indicate a null pointer as the last argument in JSValueToObject(). > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:199 > + JSObjectSetPropertyAtIndex(context, arrayObj, i, JSObjectMake(context, elements[i]->wrapperClass(), elements[i].get()), 0); Ditto. > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:508 > + bool result = (traits & [m_element _axSelectedTrait]) == [m_element _axSelectedTrait]; I suggest we inline the value of this variable into the return statement below as this variable is used exactly once in this function and I do not find that it's name helps improve the readability of this code. > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:584 > + return 0; 0 => nullptr > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:774 > + return 0; 0 => nullptr > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:802 > + // Mac programmers should not be adding more than one notification listener per element. See my remark about "Mac programmers" above.
(In reply to comment #4) Thanks. Changes made > Comment on attachment 254936 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254936&action=review > > Looks sane to me. > > > Tools/ChangeLog:11 > > + rename to more generic names. > > Minor: Please remove the indentation on this line such that the 'r' in > rename left aligns with the 'R' in rename of the previous line. > > > Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:715 > > + // Mac programmers should not be adding more than one notification listener per element. > > Did you mean to write "Mac programmers" instead of "iOS programmers" or "Mac > and iOS programmers"? I mean this comment is in an iOS-specific file. I take > it you wrote the comment using "Mac programmers" because iOS makes use of > the Mac accessibility code. > > > Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:729 > > + // Mac programmers should not be trying to remove a listener that's already removed. > > Ditto. > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:142 > > + return 0; > > 0 => nullptr > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:151 > > + return 0; > > Ditto. > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:195 > > + JSValueRef arrayResult = JSObjectMakeArray(context, 0, 0, 0); > > We should pass nullptr for the last two arguments instead of 0. > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:196 > > + JSObjectRef arrayObj = JSValueToObject(context, arrayResult, 0); > > We should pass nullptr instead of 0 to indicate a null pointer as the last > argument in JSValueToObject(). > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:199 > > + JSObjectSetPropertyAtIndex(context, arrayObj, i, JSObjectMake(context, elements[i]->wrapperClass(), elements[i].get()), 0); > > Ditto. > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:508 > > + bool result = (traits & [m_element _axSelectedTrait]) == [m_element _axSelectedTrait]; > > I suggest we inline the value of this variable into the return statement > below as this variable is used exactly once in this function and I do not > find that it's name helps improve the readability of this code. > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:584 > > + return 0; > > 0 => nullptr > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:774 > > + return 0; > > 0 => nullptr > > > Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:802 > > + // Mac programmers should not be adding more than one notification listener per element. > > See my remark about "Mac programmers" above.
http://trac.webkit.org/changeset/185609