WAI-ARIA landmarks no longer speak type of landmark
Created attachment 197911 [details] code
rdar://8736942
Created attachment 199330 [details] patch
Comment on attachment 199330 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=199330&action=review r=me > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1885 > switch (m_object->roleValue()) { > default: > return NSAccessibilityRoleDescription(NSAccessibilityGroupRole, [self subrole]); Is it normal to put the default: case at the beginning of the switch statement? > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:725 > + if ([axDescription length]) { > + if ([result length]) > + [result appendString:@", "]; > + [result appendString:axDescription]; > + } This code is essentially written 3 times in this patch. It could be pulled into a local static (inline) method to make this method a little more readable: static void appendStringToResult(NSMutableString *result, NSString *string) { ASSERT(result); if (![string length]) return; if ([result length]) [result appendString:@", "]; [result appendString:string]; } > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:744 > + return ([result length]) ? result : nil; Nit: Don't need the extra parenthesis here.
http://trac.webkit.org/changeset/149051
(In reply to comment #4) > (From update of attachment 199330 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=199330&action=review > > r=me > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1885 > > switch (m_object->roleValue()) { > > default: > > return NSAccessibilityRoleDescription(NSAccessibilityGroupRole, [self subrole]); > > Is it normal to put the default: case at the beginning of the switch statement? > > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:725 > > + if ([axDescription length]) { > > + if ([result length]) > > + [result appendString:@", "]; > > + [result appendString:axDescription]; > > + } > > This code is essentially written 3 times in this patch. It could be pulled into a local static (inline) method to make this method a little more readable: > > static void appendStringToResult(NSMutableString *result, NSString *string) > { > ASSERT(result); > if (![string length]) > return; > if ([result length]) > [result appendString:@", "]; > [result appendString:string]; > } > > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:744 > > + return ([result length]) ? result : nil; > > Nit: Don't need the extra parenthesis here. Thanks. Addressed all these suggestions in the checkin