[Webkit-unassigned] [Bug 230735] Add a module map file for PrivateFrameworks/WebKitLegacy

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=230735

--- Comment #23 from Ian Anderson <iana at apple.com> ---
Comment on attachment 442115
  --> https://bugs.webkit.org/attachment.cgi?id=442115
Patch

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

>> Source/WebCore/platform/ios/WebItemProviderPasteboard.h:-32
>> -struct CGSize;
> 
> Let's `#import <TargetConditionals.h>` instead of including the whole framework header, and add the `strict CGSize;` statement back.

Discussed in Slack, the compiler was complaining about not including the header in local submodule visibility mode.

>> Source/WebCore/platform/ios/wak/WAKAppKitStubs.h:-32
>> -#import <Foundation/Foundation.h>
> 
> To be explicit about why it's outside the `#if TARGET_OS_IPHONE`, I'd rather `#import <TargetConditionals.h>` here instead of moving the Foundation header.

Also discussed in Slack, we decided it's ok like this. (But if you change your mind, I'm good with adding TargetConditionals on top and leaving Foundation where it is.)

>> Source/WebKitLegacy/mac/Misc/WebDownload.h:40
>>  #import <WebKitLegacy/NSURLDownloadSPI.h>
> 
> I commented about this on the older patch, but are we _sure_ that WebDownload.h is a PUBLIC header?  The Xcode project says it's PRIVATE:
> 
> $ grep "WebDownload.h in Headers" Source/WebKitLegacy/WebKitLegacy.xcodeproj/project.pbxproj 
> 		939810770824BF01008DF038 /* WebDownload.h in Headers */ = {isa = PBXBuildFile; fileRef = 6578F5DE045F817400000128 /* WebDownload.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 				939810770824BF01008DF038 /* WebDownload.h in Headers */,
> 
> If it is really public, it should probably be fixed in the Xcode project, too.

It's a public header in WebKit on macOS, and a private header in WebKitLegacy. Very confusing.

>> Source/WebKitLegacy/mac/WebCoreSupport/WebCreateFragmentInternal.h:30
>> +#import <Foundation/Foundation.h>
> 
> Could probably be replaced with:
> 
> @class NSAttributedString;

We could. IMO it's bad form to forward declare interfaces you don't own though. Plus everyone that includes this header is going to have to import Foundation anyway.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211022/aa3c42a3/attachment.htm>