Closed Bug 815650 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
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.
Attachment #685647 - Flags: review?(ttaubert)
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.
Attachment #685647 - Flags: review?(ttaubert) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/28dc63439824

Are there tests for the popup blocker currently?
Target Milestone: --- → Firefox 20
https://hg.mozilla.org/mozilla-central/rev/28dc63439824
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.