815650 - Show the URI spec if there is no URI host in the popup blocker UI

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch

It has always be annoying to not read "Allow pop-ups for   " when I wanted to allow pop-ups for a "file://". This patch will show the uri.spec if uri.host does not exist. It happens that this is equivalent to uri.origin and this is what will be used after we land bug 815640.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 685647 [details] [diff] [review]
Patch

Review of attachment 685647 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +495,5 @@
>        var pm = Services.perms;
>        if (pm.testPermission(uri, "popup") == pm.ALLOW_ACTION) {
>          // Offer an item to block popups for this site, if a whitelist entry exists
>          // already for it.
> +        let blockString = gNavigatorBundle.getFormattedString("popupBlock", [uri.host != "" ? uri.host : uri.spec]);

Nit: we can do [uri.host || uri.spec]

@@ +501,5 @@
>          blockedPopupAllowSite.setAttribute("block", "true");
>        }
>        else {
>          // Offer an item to allow popups for this site
> +        let allowString = gNavigatorBundle.getFormattedString("popupAllow", [uri.host != "" ? uri.host : uri.spec]);

Same here.

Comment 2

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/28dc63439824

Are there tests for the popup blocker currently?

Comment 3

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/28dc63439824