278357 - [FIX]Add flag to nsIWebNavigation::LoadURI for the popup behaviour

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect, P2)

Description

[Crispin Flowerday (not receiving bug mail)]

Spun off from bug 276482 comment 6

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

When this is done, use of nsAutoPopupStatePusher in the gtkmozembed widget can
be removed.

Also, should this bug block bug 99625 (Freeze nsiWebNavigation)?

Comment 1

[Darin Fisher]

nsIWebNavigation is effectively frozen, sorry.  We can't afford to break binary
compatibility with it at this point.  We haven't marked it frozen because we are
still in the process of revising the comments on it.  If you need a new method,
then we need a new interface (nsIWebNavigation2 or something like that).

Comment 2

[Boris Zbarsky [:bzbarsky]]

Darin, I was thinking this would just be a load flag...

nsIWebNavigation as it stands is an incredibly poor interface (witness the fact
that we can't even use it for our own link click handling!).  So freezing it
as-is is rather sad.  :(

Comment 3

[Darin Fisher]

I agree that it is sad to freeze the interface as is.  We should create a good
nsIWebNavigation2 and freeze that too.  I'm fine adding a load flag to
nsIWebNavigation... but, the summary of this bug says "method" ;-)

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, summary is a bit off.  See the quote in comment 0.  ;)

Comment 5

[Crispin Flowerday (not receiving bug mail)]

Hmm, sorry about the summary, hopefully this is more accurate

Comment 6

[Andrew Schultz]

confirming

Comment 7

[Darin Fisher]

Can someone please spec out this new flag?  It's not clear to me what it should
be called and how it should be implemented, but I'm willing to do the work to
expose it if someone describes it precisely.  Thanks!

Comment 8

[Boris Zbarsky [:bzbarsky]]

The flag should be something like "LOAD_FLAG_ALLOW_POPUPS" and the impl should
be to place the code from bug 276482 at the top of LoadURI, I think.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Ergh....  this completely slipped off the radar.

Crispin, Darin, Benjamin, is this something we want for 1.8?  I'm rather
thinking yes....

Comment 10

[Crispin Flowerday (not receiving bug mail)]

It would be nice to have in 1.8, although sadly I have zero time to cook up a
patch :-(

Comment 11

[Benjamin Smedberg]

Not going to block on this, but we'd take a patch that wasn't too risky.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so, say

We really dropped the ball on the branch part of this.  :(

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 279132 [details] [diff] [review]
Like so, say

Requesting 1.9 approval. Patch is pretty safe and makes embeddors lives happier.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Checked in.