272389 - javascript bookmarklet popups blocked - associated with visited page

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[J. Lasser]

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. ;-)

Comment 1

[Mike Pinkerton (not reading bugmail)]

you sure this worked in 0.8.1? We've got similar bugs that have been around for
a while.

Comment 2

[Mike Pinkerton (not reading bugmail)]

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.

Comment 3

[Mike Pinkerton (not reading bugmail)]

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?

Comment 4

[Mike Pinkerton (not reading bugmail)]

we think this is caused by bug 252326. works in ff because they allow popups
when you click on a bookmark.

Comment 5

[Mike Pinkerton (not reading bugmail)]

-> geoff

jst sez: "add a nsAutoPopupStatePusher popupStatePusher(window, openAllowed) in
the scope where you load bookmarks and that should fix it"

Comment 6

[Mike Pinkerton (not reading bugmail)]

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!

Comment 7

[Mike Pinkerton (not reading bugmail)]

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.

Comment 8

[Mike Pinkerton (not reading bugmail)]

Attachment: trunk patch

trunk patch, branch will come tomorrow, i don't have a tree to verify here at
home.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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?

Comment 10

[Mike Pinkerton (not reading bugmail)]

it may very well be a dupe, yes.

Comment 11

[Mike Pinkerton (not reading bugmail)]

*** Bug 264413 has been marked as a duplicate of this bug. ***

Comment 12

[Mike Pinkerton (not reading bugmail)]

Attachment: patch for branch

Comment 13

[Mike Pinkerton (not reading bugmail)]

Attachment: updated trunk patch

fixes two issues with the original trunk patch

Comment 14

[Mike Pinkerton (not reading bugmail)]

landed on branch

Comment 15

[Mike Pinkerton (not reading bugmail)]

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?

Comment 16

[Josh Aas]

OK - so you want "updated trunk patch" landed?

Comment 17

[Mike Pinkerton (not reading bugmail)]

yes, let's land the updated trunk patch and be done with it.

Comment 18

[Mike Pinkerton (not reading bugmail)]

trunk patch landed