WebKit policy delegate should indicate ongoing user navigations. This includes carrying the "was this load the result of a user gesture?" carrying forward over redirects and subsequent main frame navigations. <rdar://problem/21025301>
Title change: WebKit policy delegate should suggest if a navigation should be allowed to open URLs externally
Two further patches: 1 - Add a "should open URLs externally" flag to DocumentLoader. The flag gets there from FrameLoadRequest. The flag is propagated out to WebKit navigation delegate API. In step 1, the flag will only ever be set from the WKWebView loadRequest: API 2 - Propagate that flag from one DocumentLoader to its descendant DocumentLoader under certain circumstances. Also, WebCore will sometimes decide to set the flag on a DocumentLoader by itself when a navigation meets certain criteria (Such as the navigation is in the main frame, and is the result of a user gesture)
Step 1 mentioned above tracked in https://bugs.webkit.org/show_bug.cgi?id=145417
Created attachment 254018 [details] Patch v1
Comment on attachment 254018 [details] Patch v1 Nevermind on the review for now - Obviously the tests aren't in good shape yet.
Comment on attachment 254018 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=254018&action=review This is missing a change to Source/WebKit/ios/WebView/WebPDFViewPlaceholder.mm and this patch causes lots of crashes. You're probably dereferencing a null pointer somewhere. > Source/WebCore/loader/FrameLoader.cpp:1326 > RefPtr<DocumentLoader> loader = m_client.createDocumentLoader(request, defaultSubstituteDataForURL(request.url())); Does createDocumentLoader always succeed? > Source/WebCore/page/DOMWindow.cpp:2139 > + Frame* activeFrame = activeWindow.frame(); > + if (!activeFrame) > + return nullptr; This isn't used. Is it helpful to return early if there is no active frame?
(In reply to comment #6) > Comment on attachment 254018 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254018&action=review > > This is missing a change to > Source/WebKit/ios/WebView/WebPDFViewPlaceholder.mm and this patch causes > lots of crashes. You're probably dereferencing a null pointer somewhere. That's why I pulled it from review :) > > > Source/WebCore/loader/FrameLoader.cpp:1326 > > RefPtr<DocumentLoader> loader = m_client.createDocumentLoader(request, defaultSubstituteDataForURL(request.url())); > > Does createDocumentLoader always succeed? Yup.
Created attachment 254030 [details] Patch v2 This is for EWS and, if it passes, for review. But not yet for landing, as I haven't updated the ChangeLogs yet.
Messed up the iOS build fix, and now that windows EWS is finally helping, will fix that one too. Rest of the patch is still reviewable at this point.
Created attachment 254070 [details] Patch v3 - More build fixing
Created attachment 254073 [details] Patch v4 - More correct build fixing
r=me. Media and font test failures are unrelated.
The test failures on mac/mac-wk2 are unrelated (Those tester bots are in a non-green status right now, anyways) I'll fix up the ChangeLogs then land.
http://trac.webkit.org/changeset/185111
Comment on attachment 254073 [details] Patch v4 - More correct build fixing Other interesting test cases: (1) Do a top-level (Safari-like) navigation that sets the flag to true, then another that sets it to false. Flag should be false. (2) Do a top-level (Safari-like) navigation that sets the flag to false, then another that sets it to true. Flag should be true. (3) Do a top-level (Safari-like) navigation that sets the flag to false, then a programmatic click event on a link. Flag should be false.
(In reply to comment #15) > Comment on attachment 254073 [details] > Patch v4 - More correct build fixing > > Other interesting test cases: > > (1) Do a top-level (Safari-like) navigation that sets the flag to true, then > another that sets it to false. Flag should be false. > > (2) Do a top-level (Safari-like) navigation that sets the flag to false, > then another that sets it to true. Flag should be true. > > (3) Do a top-level (Safari-like) navigation that sets the flag to false, > then a programmatic click event on a link. Flag should be false. Great suggestions. Following up in https://bugs.webkit.org/show_bug.cgi?id=145558