326784 - add a notification that a newly-opened window has been initialized

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Brian Ryner (not reading)]

Currently we have the "domwindowopened" notification that fires immediately after a new window has been created.  It would be useful to be able to inspect properties of the window such as the opener, name, and size, which have not yet been set.  Since "domwindowopened" is ingrained into the behavior of a few interface methods (nsPIWindowWatcher, nsIBaseWindow, nsIAppShellService), I'd like to add a new notification that would be suitable for MOZILLA_1_8_BRANCH.

Comment 1

[Brian Ryner (not reading)]

Attachment: patch

(Treat this as an either-or review request, it probably doesn't require two levels of review).

I called the new notification "domwindowready" for lack of something better.  It's a bit suboptimal in that it doesn't communicate the fact that the window is a newly-opened toplevel window.  I think as long as it's documented (where's a good place for that?) it should be ok.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> in that it doesn't communicate the fact that the window is a newly-opened
> toplevel window

Is that the intent of this notification?  If so, it's not being achieved with that patch.  For example, it will fire when loads are diverted into tabs or current windows via nsIWindowProvider...

Comment 3

[Brian Ryner (not reading)]

(In reply to comment #2)
> > in that it doesn't communicate the fact that the window is a newly-opened
> > toplevel window
> 
> Is that the intent of this notification?  If so, it's not being achieved with
> that patch.  For example, it will fire when loads are diverted into tabs or
> current windows via nsIWindowProvider...
> 

Will checking windowIsNew take care of this?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, I think so.  Perhaps call the notification "newtoplevelwindowisready" to address your other concern?

Comment 5

[Brian Ryner (not reading)]

Attachment: new patch

Ok.  "newtoplevelwindowisready" is pretty hard to read, so I changed over to the hyphenated style that's often used for observer notifications.  It's called "toplevel-window-ready", with the understanding that a window only "becomes ready" when it's initially created.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 211525 [details] [diff] [review]
new patch

Seems reasonable, if we change "dispatches" to "may dispatch" in the IDL.  Otherwise we're changing a frozen interface (in case someone overrides the window watcher contract).

Comment 7

[Brian Ryner (not reading)]

checked in, thanks for the review.

Comment 8

[Boris Zbarsky [:bzbarsky]]

So is this going on MOZILLA_1_8_BRANCH? If so, it landing there should block bug 266371 landing there.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 211525 [details] [diff] [review]
new patch

Just don't forget the comment change!