Closed Bug 278357 Opened 16 years ago Closed 13 years ago

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


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



(Not tracked)



(Reporter: crispin, Assigned: bzbarsky)


(Blocks 1 open bug)



(1 file)

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)?
Blocks: 99625
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).
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.  :(
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" ;-)
Yeah, summary is a bit off.  See the quote in comment 0.  ;)
Hmm, sorry about the summary, hopefully this is more accurate
Summary: Add method to nsIWebNavigation to enable popups → Add flag to nsIWebNavigation::LoadURI for the popup behaviour
Ever confirmed: true
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!
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.
Blocks: 275071
Ergh....  this completely slipped off the radar.

Crispin, Darin, Benjamin, is this something we want for 1.8?  I'm rather
thinking yes....
It would be nice to have in 1.8, although sadly I have zero time to cook up a
patch :-(
Flags: blocking1.9?
Not going to block on this, but we'd take a patch that wasn't too risky.
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Attached patch Like so, saySplinter Review
We really dropped the ball on the branch part of this.  :(
Assignee: adamlock → bzbarsky
Attachment #279132 - Flags: superreview?(jst)
Attachment #279132 - Flags: review?(cbiesinger)
Priority: -- → P2
Summary: Add flag to nsIWebNavigation::LoadURI for the popup behaviour → [FIX]Add flag to nsIWebNavigation::LoadURI for the popup behaviour
Target Milestone: --- → mozilla1.9 M8
Attachment #279132 - Flags: review?(cbiesinger) → review+
Attachment #279132 - Flags: superreview?(jst) → superreview+
Comment on attachment 279132 [details] [diff] [review]
Like so, say

Requesting 1.9 approval. Patch is pretty safe and makes embeddors lives happier.
Attachment #279132 - Flags: approval1.9?
Attachment #279132 - Flags: approval1.9? → approval1.9+
Checked in.
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.