160212 - Embedding needs flag to distinguish popup windows (unrequested windows) from regular windows

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[moconnell]

Need a chrome flag to determine if a window being created in CreateChromeWindow in 
CWindowCreator is for an unrequested window.

The purpose of distinguishing is two fold. First, does the window need to be built using 
an embedding application native window (this window has controls on it for features 
dealing with history, bookmarking, etc). Second, should the window be excluded from the 
embedding applications URL history.

Other embedding applications (Chimera) accomplished a similar task by exposing a new 
API in nsPIDomWindow.h. But this probably isn't the preferred solution.

Comment 1

[Conrad Carlen (not reading bugmail)]

Chimera uses the code feature added from bug 136985 to determine whether a
window is an unrequested pop-up. I want something so that we dont have to expose
something like nsIPIDomWindow in order for embeddors to know this. While the
chrome flags passed to CreateChromeWindow describe *what* sort of window to
create, that isn't enough. The embeddor needs to know the context of the window
being opened (as part of a load or timeout task) I would rather we have a way of
explicitly telling the impl of nsIWindowCreator that it's making an unrequested
pop-up. What I have in mind in this:

Have window watcher use nsIPIDOMWindow to determine whether the window it's
asking to be created thru nsIWindowCreator is an unrequested pop-up.
If so,
(1) add this as a bit in the chrome flags it passes to
nsIWindowCreator::CreateChromeWindow(). The problem here is that the flags are
defined in nsIWebBrowserChrome which is frozen. Adding new bits, as long as the
meaning of other bits is arguably OK.
(2) If window watcher determines that the window it's asking to be created is an
unrequested popup, it QI's the impl of nsIWindowCreator to a new iface
(nsIWindowCreator2) and calls its method CreateVilePopupWindow(), which most
embedding apps will want to consult a pref and refuse to create it. If the QI
fails, it just calls nsIWindowCreator::CreateChromeWindow and the embedding app
unknowingly creates the vile popup.

Opinions on how to deal with this?

Comment 2

[Conrad Carlen (not reading bugmail)]

Attachment: patch

Patch which creates nsIWindowCreator2

Comment 3

[Conrad Carlen (not reading bugmail)]

Attachment: Changes to PPEmbed resources to pop the alert dialog (.rsrc.sit file)

Comment 4

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 93575 [details] [diff] [review]
patch

r=pink

Comment 5

[Conrad Carlen (not reading bugmail)]

Changing summary and component because this is true for all platforms.

Comment 6

[Johnny Stenback (:jst)]

Could we see this change attached as text file(s) here please?

Comment 7

[Mike Pinkerton (not reading bugmail)]

jst, uh, no, because the resource changes are binary.

Comment 8

[Johnny Stenback (:jst)]

Oh, duh, I thought that was the patch <mehidesincornerfeelingstupid/>

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 93575 [details] [diff] [review]
patch

sr=jst

Comment 10

[Conrad Carlen (not reading bugmail)]

Thanks, guys, for quick reviews :-)

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 93575 [details] [diff] [review]
patch

Approved for trunk AND branch.	Change mozilla1.0.1+ to fixed1.0.1 when fixed
on branch.

Comment 12

[Conrad Carlen (not reading bugmail)]

Checked into branch.

Comment 13

[Conrad Carlen (not reading bugmail)]

Checked into trunk.

Comment 14

[Jaime Rodriguez, Jr.]

mdunn: pls verify this as fixed on the 1.0 branch, then replace the "fixed1.0.1"
keyword, with "verified1.0.1". thanks!

Comment 15

[Michael Dunn]

To ashish for bug verification

Comment 16

[Ashish Bhatt]

marking as verified