Bug 165502 – [iOS] WKWebView should not allow app links to be opened on back or forward navigation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chelsea Pugh]

WKWebView should not allow app links to be opened on back or forward navigation

[Chelsea Pugh]

Created attachment 296356 [details]
Patch for [iOS] WKWebView should not allow app links to be opened on back or forward navigation

[WebKit Commit Bot]

Attachment 296356 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/ShouldNotOpenAppLinksOnBackForwardNavigation.mm:30:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chelsea Pugh]

Created attachment 296357 [details]
Patch (with style fixed) for [iOS] WKWebView should not allow app links to be opened on back or forward navigation

[Sam Weinig]

Comment on attachment 296357 [details]
Patch (with style fixed) for [iOS] WKWebView should not allow app links to be opened on back or forward navigation

I wonder if a better way to do this would be to update API::NavigationAction::shouldOpenAppLinks() to check that navigationType is not NavigationType::BackForward.  I'm actually not sure, Dan?

(In reply to comment #4)
> Comment on attachment 296357 [details]
> Patch (with style fixed) for [iOS] WKWebView should not allow app links to
> be opened on back or forward navigation
> 
> I wonder if a better way to do this would be to update
> API::NavigationAction::shouldOpenAppLinks() to check that navigationType is
> not NavigationType::BackForward.  I'm actually not sure, Dan?

That’s possible, but that would be encoding (admittedly trivial) policy logic into what is currently a data container. I think, however, that WebPageProxy::decidePolicyForNavigationAction, which is where we currently initialize the NavigationAction, would be a good place to implement this policy. I agree that setting the m_shouldSuppressAppLinksInNextNavigationPolicyDecision in advance is unnecessarily complex and fragile.

[Sam Weinig]

(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 296357 [details]
> > Patch (with style fixed) for [iOS] WKWebView should not allow app links to
> > be opened on back or forward navigation
> > 
> > I wonder if a better way to do this would be to update
> > API::NavigationAction::shouldOpenAppLinks() to check that navigationType is
> > not NavigationType::BackForward.  I'm actually not sure, Dan?
> 
> That’s possible, but that would be encoding (admittedly trivial) policy
> logic into what is currently a data container. I think, however, that
> WebPageProxy::decidePolicyForNavigationAction, which is where we currently
> initialize the NavigationAction, would be a good place to implement this
> policy. I agree that setting the
> m_shouldSuppressAppLinksInNextNavigationPolicyDecision in advance is
> unnecessarily complex and fragile.

One thing I am also not sure of is what history.back()/history.forward()/history.go() should/would here. Probably worth testing.

[Chelsea Pugh]

Created attachment 296457 [details]
v2 patch for [iOS] WKWebView should not allow app links to be opened on back or forward navigation

[WebKit Commit Bot]

Comment on attachment 296457 [details]
v2 patch for [iOS] WKWebView should not allow app links to be opened on back or forward navigation

Clearing flags on attachment: 296457

Committed r209573: <http://trac.webkit.org/changeset/209573>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.