355633 - depending on the folder view, autosubscribing to a news:// url from the browser can cause tbird to open new 3 pane windows until the system is out of resources

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

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

depending on the folder view, autoscribing to a news:// url from the browser can cause tbird to open new 3 pane windows until the system is out of resources

try this!

1)  make tbird your default news handler (note, I'm able to do this on mac?  see bug #355632)
2)  switch to the recent view.  my recent view doesn't have any news servers or news groups.  note, I do have news://news.mozilla.org/mozilla.dev.apps.firefox in my "all view"
3)  in your browser, load news://news.mozilla.org/mozilla.dev.tech.xul

this causes tbird to start opening 3 panes until it runs out of resources.

if I hop in there and switch to the "all view", it will stop doing opening 3 panes.

Comment 1

[Scott MacGregor]

I see this too.

Comment 2

[Mike Cowperthwaite]

xref bug 181157.

Comment 3

[Scott MacGregor]

I think this is more of a blocker than a nice to have.

Comment 4

[Scott MacGregor]

The subscription succeeds, but then we end up being unable to select it in the folder pane and this seems to make us try to subscribe again by opening a new window with the same argument. 

I see a couple options here:

1) When subscribing to a news url, we open a new 3-pane window currently to handle the subscription. We could force the folder views to All when opening this window, thus avoiding the problem and the user sees the added folder in the folder pane.

2) We could try to make our code handle the case where we try to select a folder that's not in the folder pane. In this scenario, you'd now be subscribed but we wouldn't show it in the folder pane.

or if we want to a more complex solution (i.e. harder :)):

3) We automatically add the newly subscribed folder to the current view. That means marking it as a favorite, a recent folder or a folder with unread (even if it doesn't have any unread).

Comment 5

[David :Bienvenu]

Even when this succeeds because we're in the normal folder view, I see us opening 3 additional 3-pane windows, when one would do.  I'm still having trouble finding the code that's causing all these extra windows to open.

Comment 6

[Scott MacGregor]

David, when I was doing my analysis in comment 4, I believe the loop was coming from:

function loadStartFolder(initialUri)

because we end up in the exception handler because we can't select the folder uri we were given since it isn't in the view.

Comment 7

[David :Bienvenu]

thx, Scott. I'm torn about how to fix this. I don't think we should be opening up a second 3-pane at all. But in any case, even if you're in the normal folder view, I don't want us to open up four windows, so I'm thinking I just want to avoid the exception handler in this case. But I'm not sure if we're counting on this exception handling for this to work at all...

Comment 8

[David :Bienvenu]

Attachment: proposed fix

the "check if a folder has a parent" part prevents the runaway opening of new windows - if the folder has a parent, then it exists in the hierarchy, and we don't need to run a url to subscribe to it.

The change to nsNNTPService.cpp prevents the third (and fourth?) windows from getting opened. I checked that the x-application-newsgroup-listids url still works, to remove expired articles, and we no longer open a new window to run those urls.

I tried clicking on the "remove expired articles" link in a stand-alone message window, and it worked, though you had to select the newsgroup in the 3-pane ui yourself to see it.
There might be issues with this patch, but so far, it's working a lot better than it used to, for me.

Comment 9

[Scott MacGregor]

Comment on attachment 254679 [details] [diff] [review]
proposed fix

awesome!

Comment 10

[David :Bienvenu]

fixed on trunk and branch 

Comment 11

[[e-mail]]

Do we need something for SeaMonkey? We don't have that loadURL code.

Comment 12

[Mike Cowperthwaite]

What's the practical effect of these lines?
+            if (msgWindow)
+              msgWindow->SelectFolder(uriStr);

Which window is having its folder selected, and what does that mean?


(In reply to comment #11)
> Do we need something for SeaMonkey? We don't have that loadURL code.

I'm pretty sure there's an existing bug about news-URL subscriptions opening new 3-panes in the suite, but I couldn't locate it.

Comment 13

[David :Bienvenu]

It tries to select the folder in the topmost msg window's folder pane (since you clicked on a link, it's s a good bet that the window is the top most msg window).

Neil, I'm not sure about SeaMonkey - but if you don't have the loadUrl code or something like it, then you wouldn't have this bug of infinite windows opening...

Comment 14

[Mike Cowperthwaite]

By "select the folder" do you mean the newsgroup should be selected and loaded into the Thread pane, when I click on a news: link in the Message pane?
I'm not seeing that happen.

If the window that's been clicked is a standalone message window, then what?


I'm sorry if I'm being obtuse here, but this architecture is really difficult to comprehend even when the terminology is consistent.

Comment 15

[David :Bienvenu]

Select the folder, if possible, so it won't happen in the stand-alone message window case, or the case of favorites folder view w/o the folder in it...

I only tried this in the case of clicking on a news: link from an other app, in my case, Firefox.

Comment 16

[Mike Cowperthwaite]

Opening a news link from an external source is opening one additional 3pane if one is already open -- it's not clear from comment 7 & 8 if that's expected -- and the new 3pane has the same folder view as the original.

In the Recent case, the subscribed group becomes recent if it wasn't already; in the Favorite case, it remains undisplayed.  (I'm not sure what happens for an Unread view if the newsgroup has no items.)

The new window shifts to the group, if the group is part of the current folder view.  Unless the subscription is cancelled, in which case the area left of the Folder pane becomes completely blank.


(In reply to comment #12)
> I'm pretty sure there's an existing bug about news-URL subscriptions opening
> new 3-panes in the suite, but I couldn't locate it.

Bug 129574.  Also, for a number of windows opening, bug 181157 -- but I don't know if that behavior still exists in current Seamonkey.

Comment 17

[David :Bienvenu]

yes, the one additional 3-pane window opening is a known issue - it's questionable how bad a bug that is - changing the selection in the 3-pane could be undesirable as well.