Bug 210620 – Add a new 'limitNavigationsToAppBoundDomains' property to WKWebViewConfiguration

• Component: WebKit2
• Status: RESOLVED FIXED

[Brent Fulgham]

Update WKWebViewConfiguration, APIPageConfiguration, and WebPageProxy with a new flag indicating that the Developer would like to limit navigations of this web view to the set of app-bound domains specified in the Info.plist for the application.

[Brent Fulgham]

<rdar://problem/61903225>

[Brent Fulgham]

Created attachment 396710 [details]
Patch

[Kate Cheney]

I think this looks good.

[Andy Estes]

Comment on attachment 396710 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:1256
> +    _limitsNavigationToAppBoundDomains = !ignoresAppBoundDomains;

Seems odd that setting one configuration property has the side effect of changing another one. If these two properties are mutually exclusive, should have a single enum property instead with possible values of `ignoresAppBoundDomains` and `limitsToAppBoundDomains`?

[Brent Fulgham]

Comment on attachment 396710 [details]
Patch

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

>> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:1256
>> +    _limitsNavigationToAppBoundDomains = !ignoresAppBoundDomains;
> 
> Seems odd that setting one configuration property has the side effect of changing another one. If these two properties are mutually exclusive, should have a single enum property instead with possible values of `ignoresAppBoundDomains` and `limitsToAppBoundDomains`?

One is internal SPI that is used to gate several things. I just wanted to make sure that when the SPI was used, this new flag wasn't left in an inconsistent state.

[Andy Estes]

Comment on attachment 396710 [details]
Patch

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

>>> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:1256
>>> +    _limitsNavigationToAppBoundDomains = !ignoresAppBoundDomains;
>> 
>> Seems odd that setting one configuration property has the side effect of changing another one. If these two properties are mutually exclusive, should have a single enum property instead with possible values of `ignoresAppBoundDomains` and `limitsToAppBoundDomains`?
> 
> One is internal SPI that is used to gate several things. I just wanted to make sure that when the SPI was used, this new flag wasn't left in an inconsistent state.

But this just changes the value reflected by the API, not how it's enforced (but maybe that's for a follow-up). Anyway, I'd be in favor of not changing a value set by the API/SPI client but just documenting what the behavior is when there is a conflict between the two configuration settings.

[EWS]

Committed r260228: <https://trac.webkit.org/changeset/260228>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396710 [details].

[Brent Fulgham]

Reopening because I realized the setter wasn't correct.

[Brent Fulgham]

Created attachment 396729 [details]
Patch for landing

[Brent Fulgham]

Comment on attachment 396710 [details]
Patch

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

>>>> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:1256
>>>> +    _limitsNavigationToAppBoundDomains = !ignoresAppBoundDomains;
>>> 
>>> Seems odd that setting one configuration property has the side effect of changing another one. If these two properties are mutually exclusive, should have a single enum property instead with possible values of `ignoresAppBoundDomains` and `limitsToAppBoundDomains`?
>> 
>> One is internal SPI that is used to gate several things. I just wanted to make sure that when the SPI was used, this new flag wasn't left in an inconsistent state.
> 
> But this just changes the value reflected by the API, not how it's enforced (but maybe that's for a follow-up). Anyway, I'd be in favor of not changing a value set by the API/SPI client but just documenting what the behavior is when there is a conflict between the two configuration settings.

I'll land a follow-up fix addressing that.

[Brent Fulgham]

Created attachment 396730 [details]
Patch for landing

[EWS]

Committed r260232: <https://trac.webkit.org/changeset/260232>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396730 [details].

[EWS]

Patch 396729 does not build