Bug 196284 – [Cocoa] Add new API around WKWebpagePreferences in WKNavigationDelegate and WKWebViewConfiguration

• Component: WebKit2
• Status: RESOLVED FIXED

[Wenson Hsieh]

More WIP towards <rdar://problem/47228232>

[Wenson Hsieh]

Created attachment 366042 [details]
Patch

[Wenson Hsieh]

Created attachment 366313 [details]
Patch

[Tim Horton]

Comment on attachment 366313 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKNavigationDelegate.h:87
> + @discussion If you do not implement this method,
> + -webView:decidePolicyForNavigationAction:decisionHandler: will be called

More importantly, if you DO implement this one, the historical one won't be called.

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.h:133
> +/*! @abstract The set of default web page preferences to use when loading and rendering content.

Just putting it out there...  we went with Webpage because we write it webpage. If we wrote it "web page" it would be WebPage. So you've got the non-camel-cased version wrong here.

[Wenson Hsieh]

Comment on attachment 366313 [details]
Patch

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

>> Source/WebKit/UIProcess/API/Cocoa/WKNavigationDelegate.h:87
>> + -webView:decidePolicyForNavigationAction:decisionHandler: will be called
> 
> More importantly, if you DO implement this one, the historical one won't be called.

Good point — changed the @discussion here to make that clearer.

>> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.h:133
>> +/*! @abstract The set of default web page preferences to use when loading and rendering content.
> 
> Just putting it out there...  we went with Webpage because we write it webpage. If we wrote it "web page" it would be WebPage. So you've got the non-camel-cased version wrong here.

Old habits die hard :|

Replaced "web page" with "webpage" here, and in other places (including WKNavigationDelegate.h)

[Wenson Hsieh]

Created attachment 366454 [details]
Rebase on trunk

[Wenson Hsieh]

>     /bin/sh -c /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release-iphoneos/WebKit.build/Script-5DF408C6131DD49700130071.sh
> error: WebKit.framework/Headers/WKWebpagePreferences.h included <WebKitAdditions/WKWebpagePreferencesAdditionsBefore.h> but I could not find a header of that name!
> error: WebKit.framework/Headers/WKWebpagePreferences.h included <WebKitAdditions/WKWebpagePreferencesAdditionsAfter.h> but I could not find a header of that name!
> 
> ** BUILD FAILED **
> 
> 
> The following build commands failed:
> 	PhaseScriptExecution Check\ For\ Framework\ Include\ Consistency /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release-iphoneos/WebKit.build/Script-5DF408C6131DD49700130071.sh

Well, good to know "Check For Framework Include Consistency" is working as intended :/

It seems `WK_FRAMEWORK_HEADER_POSTPROCESSING_DISABLED=YES` for certain build configurations, which includes macOS prior to 10.13. Perhaps I need to pull my "Replace WebKitAdditions includes" script out of postprocess-framework-headers.sh, and into either a separate build phase, or run it alongside postprocess-framework-headers.sh in the "Postprocess Framework Headers" build phase.

[Wenson Hsieh]

> It seems `WK_FRAMEWORK_HEADER_POSTPROCESSING_DISABLED=YES` for certain build
> configurations, which includes macOS prior to 10.13. Perhaps I need to pull

*prior to 10.14.

[Wenson Hsieh]

Created attachment 366543 [details]
Patch

[Tim Horton]

Comment on attachment 366543 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKNavigationDelegate.h:89
> + Otherwise, -webView:decidePolicyForNavigationAction:decisionHandler: will be
> + called if it is implemented.

What is this sentence for? I think you just need the previous one

[Wenson Hsieh]

Comment on attachment 366543 [details]
Patch

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

>> Source/WebKit/UIProcess/API/Cocoa/WKNavigationDelegate.h:89
>> + called if it is implemented.
> 
> What is this sentence for? I think you just need the previous one

Sounds good — removed the "Otherwise, …" sentence.

[Wenson Hsieh]

Created attachment 366576 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 366576 [details]
Patch for landing

Clearing flags on attachment: 366576

Committed r243787: <https://trac.webkit.org/changeset/243787>