47981 - news://host/group (and news:group) urls launch new 3 pane window

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P3)

Description

[Dave Coyle]

[build 2000080712]

Clicking on HTML links to newsgroups e.g.
news://news.mozilla.org/netscape.public.mozilla.webtools or
news://news.mozilla.org/netscape.public.mozilla.mail-news launches a new Mail
window each time, even though one was already open. Pasting above url into
browser location bar produces the same behavior.

Clicking link or pasting url also launches blank 'account wizard' dialog box.
Dialog box reappears immediately after restarting Mail.

Comment 1

[Gregory McLean]

Confirmed 2000080804 Linux

Comment 2

[Gregory McLean]

Really confirm it...

Comment 3

[R.K.Aa.]

the "blank" wizard is bug 47123

Comment 4

[lchiang]

future per mail triage

Comment 5

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

accepting all "future" bugs to get them off my "new" bug radar.

Comment 6

[[e-mail] (gone - use [e-mail] instead)]

Seeing this also on Windows 2000, build 2000121608.  Setting Platform/OS to ALL.

Comment 7

[Håkan Waara]

This is really annoying. Fix coming up soon.

Comment 8

[Håkan Waara]

Attachment: Fix

Comment 9

[Håkan Waara]

The patch uses the windowmediator to walk to windowtree, to see if there's any 
messenger window open, if there is, we focus it, if not, we open a new window.

I've done extensive testing with this patch, together with the patch in bug 
40033.

Bienvenu, can you please r= so sspitzer can sr=?

Comment 10

[David :Bienvenu]

I'll let Seth r and I'll sr, if that's OK.

Comment 11

[Håkan Waara]

Comment on attachment 54870 [details] [diff] [review]
Fix

This patch broke multiple mail window handling. I'm working on a better patch.

Comment 12

[Håkan Waara]

Actually, it looks like this is broken in trunk builds as well, so this is not
my fault.

Comment 13

[Håkan Waara]

Attachment: New fix

Comment 14

[Håkan Waara]

The new fix does only affect news urls. My last patch may have affected other 
parts of mailnews, since it was very general.

Comment 15

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

looks good, but instead of defining the static id, use the contract id.

NS_WINDOWMEDIATOR_CONTRACTID

Comment 16

[Håkan Waara]

Attachment: Patch addressing Seth's comment

Comment 17

[Håkan Waara]

New patch posted with a change to use the CONTRACTID, as suggested. Now awaiting
additional review.

Comment 18

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

nsCOMPtr<nsIWindowMediator> mediator(do_GetService(NS_WINDOWMEDIATOR_CONTRACTID));

we should check either check rv or if the mediator was null.

I'd suggest:

nsCOMPtr<nsIWindowMediator> mediator =
do_GetService(NS_WINDOWMEDIATOR_CONTRACTID, &rv);
NS_ENSURE_SUCCESS(rv,rv);

Comment 19

[Håkan Waara]

Attachment: Patch -- add and prefer NS_ENSURE_SUCCESS for safety

Comment 20

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

Comment on attachment 55076 [details] [diff] [review]
Patch -- add and prefer NS_ENSURE_SUCCESS for safety

r=sspitzer

the removal of the if (!aChannel) is safe because QI will return error on failure.

Comment 21

[Håkan Waara]

Fix checked in.

Comment 22

[[e-mail] (gone - use [e-mail] instead)]

This works great on Windows and Linux (and, great for the _1st_ time on Mac). 
Subsequent parses of either |news://news.mozilla.org/netscape.test| or
|news:netscape.test| URLs on Mac fail as follows:

Entering news:netscape.test and hitting enter results in:
* news://news.mozilla.org:119/netscape.test

Entering news://news.mozilla.org:119/netscape.test results in:
* news://news.mozilla.org:119/netscape.test

On Mac, we still don't open up a new window (which is what this bug was about),
but we're still doing something weird in the URL field [extractFromPort()], perhaps?

On Windows and Linux, this is fixed because we don't open a new window and we
don't change the url field.

I'll spin up a new bug about the Mac, or maybe just note the case in bug 106553.
See also semi-related bug 40033.

Mac OS 9.1 - build 2001-10-30-08
Windows 2K - build 2001-10-30-03
RedHat 7.1 - build 2001-10-30-12

Comment 23

[[e-mail] (gone - use [e-mail] instead)]

Correction:

'Entering news://news.mozilla.org:119/netscape.test results in:
* news://news.mozilla.org:119/netscape.test'

Should read:

'Entering news://news.mozilla.org/netscape.test results in:
* news://news.mozilla.org:119/netscape.test'

I think the port should be transparent to the user, since 119 is the default for
NNTP.