| Summary: | [iOS] WKWebView should not allow app links to be opened on back or forward navigation | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Chelsea Pugh <cpugh> |
| Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | commit-queue, cpugh, mitz, sam |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Attachments: | |||
|
Description
Chelsea Pugh
2016-12-06 17:01:24 PST
Created attachment 296356 [details]
Patch for [iOS] WKWebView should not allow app links to be opened on back or forward navigation
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.
Created attachment 296357 [details]
Patch (with style fixed) for [iOS] WKWebView should not allow app links to be opened on back or forward navigation
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. (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. Created attachment 296457 [details]
v2 patch for [iOS] WKWebView should not allow app links to be opened on back or forward navigation
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> All reviewed patches have been landed. Closing bug. |