76327 - Context menus don't work in new windows opened from a _new targeted link

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Adam Lock]

Embedding apps receive no calls to their context menu listener for new windows 
opened from a _new targetted link.

Steps to reproduce:

1. Run mfcEmbed
2. Load a page with a link such as <a href="http://www.mozilla.org/" 
target="_new">Some link</a>.
3. Right-mouse click on new window. No menus appear!

Problem appears to be because the web progress notifications go to the original 
window, not the new one. This can be traced back to nsDocShell::LoadURI where it 
is clear that load operation uses the original docshell information for the 
progress notification callbacks because the new window doesn't exist yet.

Comment 1

[Adam Lock]

Attachment: Patch adds an explicit call to add the chrome listeners during the web browser creation.

Comment 2

[Judson Valeski]

it looks like the chrome listeners are being hooked up on the first
OnProgressChange() callback. why wouldn't they just get hooked up to the new
window when OnProgressChange() gets called for the new window?

Comment 3

[Judson Valeski]

from adam (edited a bit):
"The tooltip and context menu listeners are installed when the first web
progress notification arrives. The problem seems to be that for targetted
windows, the http channel opened for the link is set to send notifications to
the original window, not the targte window. This means no progress events arrive
in the new window and the listeners never get installed. Notice also in mfcEmbed
that the progress bar never updates either for the new window.

Clicking on another link in the new window does install them correctly.

I think it may be possible to move the call to 
nsDocShellTreeOwner::AddChromeListeners() to somewhere other than the progress 
notification, such as into the nsWebBrowser::Create() just after the 
nsDocShellTreeOwner object is created."

"The patch puts an explicit call to register the listeners during creation of
the web browser.

Note that only fixes the missing menus problem. The original issue of web 
progress notifications don't go to the new window is still open. I suspect that 
for this to occur the new window would have to exist before the load operation 
started, something which doesn't happen at the moment."

IMO, we should figure out why the new window isn't getting progress.

Comment 4

[Judson Valeski]

requesting 0.9 targeting.

Comment 5

[Adam Lock]

The reason the new window doesn't get the progress notifications is because it 
doesn't exist when loading is initiated by nsDocShell::DoURILoad. Instead, the 
old docshell passes itself as the ifreq parameter to NS_OpenURI that specifies 
the notification callbacks. 

The solution is a bit tricky but may involve opening the new window first and 
passing the docshell to that instead, or perhaps changing the way notifications 
are registered so that a new window can fix them up at a later stage after it 
has opened.

Comment 6

[Judson Valeski]

opening the new window first makes most sense to me; hard change?

Comment 7

[Adam Lock]

I think there would be issues about opening the window beforehand. A chrome 
window has it's own progress notification needs and there won't be primary 
content docshell until it has loaded all of its chrome. We would need to wait 
for the content docshell before we could initiate our load.

I think it may be neater (and faster) if the uri loader could fix up the 
docshell to use for progress notification callbacks later on after the window 
had really been created. For example, somewhere after nsURILoader::GetTarget is 
called.

Anyway, back to this specific bug. Jud & Rick can I have the r= & sr= on it and 
I will check-in? This alleviates the most painful symptom, although obviously 
the cause does need addressing.

Comment 8

[Judson Valeski]

making 0.9.

r=valeski. Please open a new bug new window not getting progress callbacks.

Comment 9

[rpotts (gone)]

sr=rpotts

Comment 10

[Adam Lock]

Fix is checked in

Comment 11

[Adam Lock]

Bug 76408 covers the more general web progress notifications issue.

Comment 12

[Michael Dunn]

Correction: Changing QA contact for the Embed API bugs to David Epstein.

Comment 13

[gordon]

*** Bug 77077 has been marked as a duplicate of this bug. ***

Comment 14

[David Epstein]

context menus now appear after url change.