Bug 147426 – Don't use (Details) when exposing SPI

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Dean Jackson]

If we are simply declaring the interface and not implementing, we should use class extensions rather than categories.

[Radar WebKit Bug Importer]

<rdar://problem/22062407>

[Dean Jackson]

Created attachment 257782 [details]
Patch

[Dean Jackson]

Created attachment 257788 [details]
Patch

Comment on attachment 257788 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257788&action=review

> Source/WebCore/platform/mac/ScrollViewMac.mm:43
> +@interface NSScrollView (Details)
>  @property BOOL automaticallyAdjustsContentInsets;
>  @end

Is this declared here for building on Mavericks? If so, can you guard this category with #if __MAC_OS_X_VERSION_MIN_REQUIRED < 101000 so that it’s easy to remove later? Maybe also give the category a name that begins with “Web”.

> Source/WebCore/platform/spi/cocoa/NSButtonCellSPI.h:30
> +#if PLATFORM(MAC) && USE(APPLE_INTERNAL_SDK)

We don’t need the PLATFORM(MAC) here. The fact that the #import <AppKit/NSButtonCell.h> above works proves that this header is used only on Mac, and the __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000 is true only on Mac.

[Dean Jackson]

Comment on attachment 257788 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257788&action=review

>> Source/WebCore/platform/mac/ScrollViewMac.mm:43
>>  @end
> 
> Is this declared here for building on Mavericks? If so, can you guard this category with #if __MAC_OS_X_VERSION_MIN_REQUIRED < 101000 so that it’s easy to remove later? Maybe also give the category a name that begins with “Web”.

You are right. I originally left it in because we can't redeclare it in an extension, but it is only needed for Mavericks.

>> Source/WebCore/platform/spi/cocoa/NSButtonCellSPI.h:30
>> +#if PLATFORM(MAC) && USE(APPLE_INTERNAL_SDK)
> 
> We don’t need the PLATFORM(MAC) here. The fact that the #import <AppKit/NSButtonCell.h> above works proves that this header is used only on Mac, and the __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000 is true only on Mac.

Fixed.

[Dean Jackson]

Committed r187609: <http://trac.webkit.org/changeset/187609>