Bug 143497 – Content extensions should block popups

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alex Christensen]

We should.

[Alex Christensen]

Created attachment 250301 [details]
Patch

[Brady Eidson]

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()

[Alex Christensen]

Created attachment 250310 [details]
Patch

[Geoffrey Garen]

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.

[Alex Christensen]

(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?

[WebKit Commit Bot]

Comment on attachment 250310 [details]
Patch

Clearing flags on attachment: 250310

Committed r182511: <http://trac.webkit.org/changeset/182511>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 143507

[Alex Christensen]

This caused https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/r182511%20(3359)/results.html

[Alex Christensen]

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