Going for more reduction of GetMsgFolderFromUri calls here. messageWindow.js currently has a gCurrentFolderUri property, when it can just as easily hold a gCurrentFolder one. This patch takes care of that, along with removing a couple little-used functions. Note: This patch only applies on top of the patch in bug 455967

Neil, can you r/sr the SM parts? I had to extend jminta's patch a little because searchDialog.js changed MsgOpenNewWindowForFolder to take a folder instead of a URI, so I had to make parallel changes in SM. I have not tested them in SM at all, however. The original patch broke opening folders in new windows because it didn't change the 3 pane ui to expect a folder instead of a folderURI, so I fixed that. I could have gone further with changing gLoadStartFolderUri to gLoadStartFolder, but I got a little nervous about the news subscribe comment - I suspect we don't do autosubscribe for newsgroups that way anymore, though I'm going to try it out. If we do, we might need to go back to passing folder uri's around in that case.

subscribing to a news group by clicking on a link in a message works in TB. I'm not sure if clicking on a link to a newsgroup in a web page works in SM (w or w/o this patch)

Comment on attachment 351632 [details] [diff] [review] un-bittrotted patch, attempt not to break SM >- // argument[0] --> folder uri >+ // argument[0] --> folder > // argument[1] --> optional message key > // argument[2] --> optional email address; // Will come from aim; needs to show msgs from buddy's email address. > if ("arguments" in window) > { > // filter our any feed urls that came in as arguments to the new window... > if (window.arguments.length && /^feed:/i.test(window.arguments[0] )) You'd better unbreak Thunderbird before asking me for review ;-)

grumble, I think I'm just going to back out the part that changes the 3 pane ui to a folder and not a uri.

Comment on attachment 339371 [details] [diff] [review] patch v1 minusing - bit-rotted, and has other issues, see prev comments.

aceman, is this a simple enough patch that you can suggest someone to take it? (not suggesting you necessarily)

I could probably refresh it. But GetMsgFolderFromUri no longer exists in TB. And what is the benefit of this change? Do we need the folder itself more often than the uri? Does anybody remember?

I agree with comment 7, that this is not needed. The current task is to get rid of the RDF lookups and replace them with nsIFolderLookupService. I don't know of any plans to get rid of folder URIS, and it is perfectly legitimate to store a reference to a URI instead of the folder itself.

Maybe holding a reference to a folder will pin it in cache? Keeping just the URI as a string may not have that effect.

(In reply to :aceman from comment #9) > Maybe holding a reference to a folder will pin it in cache? Keeping just the > URI as a string may not have that effect. jminta's work was focused on de-rdfification, so I doubt if that is the original justification for this. Actually one of the reasons that I sometimes prefer urls rather than xpcom objects is that it reduces the number of references, so as to not "pin" the item that can lead to leaks or excess memory usage if not managed well. But I say that as a general statement, not having looked much at this specific issue. Generally the parent holds a strong reference to the children, and the children a weak reference to the parent (and a url is a form of weak reference). In this case, the view is the parent so it is not completely unreasonable to hold a strong reference to the folder. But if it ain't broke, why fix it? Still if there were specific reasons here I could be persuaded. But "jminta wrote a patch as part of his war on rdf, so we should try to land it" is not persuasive.

I think we can close this.