[webkit-reviews] review denied: [Bug 97301] AX: Layout tests would be easier to write if AccessibilityController could find an element by id : [Attachment 165148] Feedback addressed

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has denied Dominic Mazzoni
<dmazzoni at google.com>'s request for review:
Bug 97301: AX: Layout tests would be easier to write if AccessibilityController
could find an element by id
https://bugs.webkit.org/show_bug.cgi?id=97301

Attachment 165148: Feedback addressed
https://bugs.webkit.org/attachment.cgi?id=165148&action=review

------- Additional Comments from Timothy Hatcher <timothy at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=165148&action=review


>> Tools/DumpRenderTree/mac/AccessibilityCommonMac.h:43
>> + at interface NSString (JSStringRefAdditions)
> 
> Extra space before ( in function call  [whitespace/parens] [4]

This does seem bogus. Just ignore it — StyleQueue isn't the final say.

> Tools/DumpRenderTree/mac/AccessibilityCommonMac.h:64
> + at implementation NSString (JSStringRefAdditions)
> +
> ++ (NSString *)stringWithJSStringRef:(JSStringRef)jsStringRef
> +{
> +    if (!jsStringRef)
> +	   return nil;
> +
> +    CFStringRef cfString = JSStringCopyCFString(kCFAllocatorDefault,
jsStringRef);
> +    return [(NSString *)cfString autorelease];
> +}
> +
> +- (JSStringRef)createJSStringRef
> +{
> +    return JSStringCreateWithCFString((CFStringRef)self);
> +}
> +
> + at end

The @implementation should not be in a .h file. If it was in a .m file it would
be fine. I'm surprised this builds.

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityCommonMac.h:64
> + at implementation NSString (JSStringRefAdditions)
> +
> ++ (NSString *)stringWithJSStringRef:(JSStringRef)jsStringRef
> +{
> +    if (!jsStringRef)
> +	   return nil;
> +
> +    CFStringRef cfString = JSStringCopyCFString(kCFAllocatorDefault,
jsStringRef);
> +    return [(NSString *)cfString autorelease];
> +}
> +
> +- (JSStringRef)createJSStringRef
> +{
> +    return JSStringCreateWithCFString((CFStringRef)self);
> +}
> +
> + at end

Ditto.