276482 - opening windows using javascript: links fails

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: GTK Widget, defect)

Description

[Crispin Flowerday (not receiving bug mail)]

Since bug 257598 was fixed, opening javascript: links using the
gtk_moz_embed_load_uri() function fails to allow the javascript link to open a
new window.

To reproduce:

1) Open TestGtkEmbed
2) Go to about:config and set dom.disable_open_during_load to true
3) paste the above url into the location bar

I would expect a new window to open, but instead it is blocked.

Comment 1

[Crispin Flowerday (not receiving bug mail)]

Attachment: Proposed fix

This patch just allows popup windows when the embedding widget calls
nsIWebNavigation::LoadURI() which only happens via the gtk_moz_embed_load_url()
call.

Comment 2

[Crispin Flowerday (not receiving bug mail)]

Sorry, I mean bug 252326 (in comment #0)

Comment 3

[Marco Pesenti Gritti]

Maybe we need a way to load_url while still leaving popup blocker enabled?
See the patch landed for bug 272389. They are still leaving popup blocker
enabled for OpenInNewWindow for example...

Comment 4

[Crispin Flowerday (not receiving bug mail)]

Comment on attachment 169916 [details] [diff] [review]
Proposed fix

Marco isn't happy reviewing this as he doesn't understand the API, so
requesting from bzbarsky who r= the original patch.

I don't believe that an API to leave the popups blocked is needed. This only
affects (AFAICT) javascript links (such as bookmarklets.

Comment 5

[Crispin Flowerday (not receiving bug mail)]

bzbarsky: I'm cc'ing you to see if this is the correct fix for the gtk embedding
widget, as you gave r= to the original patch (bug 252326)

Comment 6

[Boris Zbarsky [:bzbarsky]]

So... What really ought to happen, in my opinion, is that nsIWebNavigation
should have a way of specifying to loadURI what the popup behavior should be. 
Then the popup state should be munged inside nsIWebNavigation.

It's really silly to expect all embeddors to deal with this by hand like this,
especially given the use of nsPIDOMWindow...

Comment 7

[Crispin Flowerday (not receiving bug mail)]

bzbarsky: Is this patch ok for the moment, I will happily raise another bug
about changing nsIWebNavigation, but we have had reports from users that this
doesn't work, so it is something I would like to at least have a temporary fix
in for the moment.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 169916 [details] [diff] [review]
Proposed fix

This is OK as a band-aid, but please file a followup bug on doing this right,
with a pointer to this code so it can be removed.

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 169916 [details] [diff] [review]
Proposed fix

sr=jst, but yeah, we need a followup bug to do this right. Please cc me on the
followup bug.

Comment 10

[Crispin Flowerday (not receiving bug mail)]

Passing the popup state to nsIWebNavigation has been raised as bug 278357

Comment 11

[Crispin Flowerday (not receiving bug mail)]

Comment on attachment 169916 [details] [diff] [review]
Proposed fix

I'm asking for approval, as this is a simple (although bit of a sticking
plaster) fix that allows the gtkmozembed users to open javascript bookmarklets
properly.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in embedding/browser/gtk/src/EmbedPrivate.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.cpp,v  <--  EmbedPrivate.cpp
new revision: 1.50; previous revision: 1.49
done

Comment 13

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 169916 [details] [diff] [review]
Proposed fix

a=caillon for both branches.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

1.7 branch:
Checking in embedding/browser/gtk/src/EmbedPrivate.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.cpp,v  <--  EmbedPrivate.cpp
new revision: 1.38.14.2; previous revision: 1.38.14.1
done

AVIARY_1_0_20040515_BRANCH (I hope this is the right one):
Checking in embedding/browser/gtk/src/EmbedPrivate.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.cpp,v  <--  EmbedPrivate.cpp
new revision: 1.38.18.2; previous revision: 1.38.18.1
done

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

now also checked in on AVIARY_1_0_1_20050124_BRANCH:
Checking in embedding/browser/gtk/src/EmbedPrivate.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.cpp,v  <--  EmbedPrivate.cpp
new revision: 1.38.18.1.2.1; previous revision: 1.38.18.1
done