213538 - fix #ifdef MOZ_THUNDERBIRD code in nsWebShell::OnLinkClickSync()

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[(not reading, please use [e-mail] instead)]

fix #ifdef MOZ_THUNDERBIRD code in nsWebShell::OnLinkClickSync()

this hack was my doing, but assigning to mscott, who might have more cycles than
me to fix it.

Comment 1

[Boris Zbarsky [:bzbarsky]]

From a mail I sent earlier:

The right way to do this is probably to have your nsIURIContentListener (which
should be the parent of the content listener of the main docshell in your mail
window) have a OnStartURIOpen method that, for certain protocols, punts the URI
load to the OS and aborts it.

OnStartURIOpen is called in the URILoader prior to opening a channel, thus you
should get what you want, I think....

Mailnews does something along those lines with nsMsgWindow.cpp -- see the way
that class is set as the parent content listener in nsMsgWindow::SetRootDocShell
and the way it overrides, eg, IsPreferred().

Have to be a little careful with iframes, perhaps, but this should be a much
cleaner approach.

Comment 2

[Scott MacGregor]

accepting. Thanks for filing this guys.

Comment 3

[Scott MacGregor]

Attachment: remove the ifdef from nsWebshell down into mailnews

This patch removes the docshell ifdef and moves it into
nsMsgWindow::OnStartURIOpen. There is also some more cleanup to fix a side
issue Neil and I were discussing a while ago.

Comment 4

[Scott MacGregor]

fixed!

Comment 5

[Scott MacGregor]

Unfortunately OnStartURIOpen doesn't tell you if the url originated from a link
click or not. 

Internal attempts to open chrome, jar, about:blank and even http urls (the mail
start page) need to be allowed. Link clicks for these types of urls need to be
kicked to the OS. 

To work around this instead of backing my patch out, I'm currently allowing all
jar, about:blank and chrome urls to load in the msg window's docshell. 

However this still leaves the http start page url broken.

Hopefully Boris and I can come up with an API change that will allow
OnStartURIOpen to know the origins of the url (was it a link click, etc.). 

Comment 6

[[e-mail]]

Won't nsMessenger::LoadURL force a load of the URL in the message pane?
Or is that unsuitable?

Comment 7

[Scott MacGregor]

Re-opening. I had to back this patch out. I broke mail display for thunderbird
on mac, linux and older win9x OSes. For some reason this path semi-works for win
xp modulo the issues listed in one of my earlier comments.

Comment 8

[Scott MacGregor]

Attachment: patch used to back this out (for historical purposes)

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Hopefully Boris and I can come up with an API change

The relevant interfaces are all nicely frozen.  We could extend them, of course.

> For some reason this path semi-works for win

Your patch never inits the out param for the mailnews protocols.  On windows,
stack vars get randomly inited to 0 if not inited explicitly; on other
platforms, you get random garbage in the out param, and the random garbage is
typically nonzero.  You need to set the out param to false when you don't want
the load aborted.

Comment 10

[[e-mail]]

nsMessenger::OpenURL also seems to force the load of a document into the message
pane... I don't pretend to know what the difference is. Note that links in the
loaded document correctly open in the browser. Test cases:
Tools/Javascript Console
top.opener.messenger.OpenURL('http://www.mozilla.org/');
top.opener.messenger.loadURL(null, 'http://www.mozilla.org/');

Comment 11

[[e-mail]]

Fixed by bug 226071.