We should.
Created attachment 250301 [details] Patch
Comment on attachment 250301 [details] Patch As discussed in person, this prevents the creation of windows in a lot more cases than just "popups". The check(s) should be moved to everywhere we already consult the existing popup blocker. i.e. calls to allowPopUp()
Created attachment 250310 [details] Patch
Comment on attachment 250310 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250310&action=review > Source/WebCore/page/DOMWindow.cpp:2147 > +#if ENABLE(CONTENT_EXTENSIONS) > + if (firstFrame > + && firstFrame->mainFrame().page() > + && firstFrame->mainFrame().page()->userContentController() > + && firstFrame->mainFrame().document()) { > + ResourceLoadInfo resourceLoadInfo = {URL(ParsedURLString, urlString), firstFrame->mainFrame().document()->url(), ResourceType::Popup}; > + Vector<ContentExtensions::Action> actions = firstFrame->mainFrame().page()->userContentController()->actionsForResourceLoad(resourceLoadInfo); > + for (const ContentExtensions::Action& action : actions) { > + if (action.type() == ContentExtensions::ActionType::BlockLoad) > + return nullptr; > + } > + } > +#endif Do we also need to consult the content extension before popping out to another app (say, from an itms:// link or some other custom scheme link)? I think popping out to another app is very similar to popping out to another window.
(In reply to comment #4) > Do we also need to consult the content extension before popping out to > another app (say, from an itms:// link or some other custom scheme link)? Probably. Where should this code be, and where are tests that test something similar?
Comment on attachment 250310 [details] Patch Clearing flags on attachment: 250310 Committed r182511: <http://trac.webkit.org/changeset/182511>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 143507
This caused https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/r182511%20(3359)/results.html
Used Document::completeURL with proper null checks instead of URL(ParsedURLStringTag, const String&) like is done elsewhere in DOMWindow::open Recommitted to http://trac.webkit.org/changeset/182564