Closed Bug 272389 Opened 20 years ago Closed 20 years ago

javascript bookmarklet popups blocked - associated with visited page

Categories

(Camino Graveyard :: Bookmarks, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Camino0.9

People

(Reporter: jon, Assigned: me)

References

()

Details

Attachments

(2 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.5) Gecko/20041129 Camino/0.8.2 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.5) Gecko/20041129 Camino/0.8.2 I have a javascript bookmarklet to post del.icio.us links via a popup window. (I'm using the nutr.itio.us script referenced in the URL.) However, the popup blocker ALWAYS blocks the bookmarklet from opening a window unless I unblock popups on the site I'm visiting (ie, that I'm trying to bookmark). Javascript that opens windows and is contained in a bookmark should NOT be associated with the visited page, since the javascript does not originate on that page. This worked correctly in 0.8.1, but breaks in the RCs for 0.8.2, including the most recent one as of tonight. (I had noticed misbehavior in this in an earlier RC, but didn't figure out what the problem was until just now...) Reproducible: Always Steps to Reproduce: 1. Create a nutr.icio.us posting bookmarklet via the above site. (I stuck mine in my bookmark bar, if that makes any difference, code-wise.) 2. Turn on the popup blocker 3. Visit a site you wish to bookmark via del.icio.us 4. Select the bookmarklet Actual Results: The popup blocker shows in the lower-left corner of the window, offers to unblock the site being visited. The popup window does NOT display. Expected Results: The popup window created by the javascript should open. Please let me know if I can offer any further support or explanation so that this can be debugged --- it's a MAJOR frustration, as it interferes with my blogging. ;-)
you sure this worked in 0.8.1? We've got similar bugs that have been around for a while.
ok, this did work in 081. i'm curious if moz1.7.5 works as there were many security fixes which could have affected this.
jst, we have a suspicion this regressed when all the security changes landed on the 1.7 branch around the end of october. geoff is going to try to nail it down, but is there anything you can think of that would have caused this?
Status: UNCONFIRMED → NEW
Ever confirmed: true
we think this is caused by bug 252326. works in ff because they allow popups when you click on a bookmark.
No longer depends on: 252326
-> geoff jst sez: "add a nsAutoPopupStatePusher popupStatePusher(window, openAllowed) in the scope where you load bookmarks and that should fix it"
Assignee: pinkerton → me
Blocks: 261393
jst, you said that ff allowed all bookmarks to open popups but that's not at all correct. what is the reason this works in ff? I'm also not about to allow any bookmark to open a popup window. what tidbit did you leave out of the explanation? thanks!
jst, nevermind. i figured it out. the patch, however, is MUCH bigger than i wanted it to be. I was hoping that we could bottleneck it at the main controller where we load bookmarks, but alas, the window isn't yet open in many cases and we need the nsPIDOMWindow in order for this to work. So i had to pass flags all the way down at every single place we load urls in the app so they go into CHBrowserView where we do the actual load. suckage. I'm positive this won't apply to the branch either because of the tab loading changes. Sigh. I'll do the branch patch tomorrow. Someone please check it on the trunk. It appears to work great.
Attached patch trunk patch (obsolete) (deleted) — Splinter Review
trunk patch, branch will come tomorrow, i don't have a tree to verify here at home.
Isn't this a dupe of the older bug 264413 (except this one has a patch), or are there some finer details I'm missing?
it may very well be a dupe, yes.
*** Bug 264413 has been marked as a duplicate of this bug. ***
Attachment #167544 - Flags: review?(joshmoz)
Attached patch updated trunk patch (deleted) — Splinter Review
fixes two issues with the original trunk patch
Attachment #167510 - Attachment is obsolete: true
Attachment #167544 - Flags: review?(joshmoz) → review+
landed on branch
josh, i know we talked about combining the params into a nsdictionary, but i say we do that only if we need to do this again. right now, i think it's just on the border of being ok, let's get this landed on the trunk, ok?
OK - so you want "updated trunk patch" landed?
yes, let's land the updated trunk patch and be done with it.
Target Milestone: --- → Camino0.9
trunk patch landed
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: