[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b3pre) Gecko/2008011002 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4) 1. Show Sidebar History panel. 1r. [ Error: SortInNewDirection is not defined Source File: chrome://communicator/content/history/history.js Line: 111 ] *** <http://mxr.mozilla.org/seamonkey/search?string=SortInNewDirection> [ SortInNewDirection /xpfe/global/resources/content/nsTreeSorting.js, * line 52 -- function SortInNewDirection(direction) /suite/common/history/history.js, * line 111 -- SortInNewDirection(find_sort_direction(find_sort_column())); /suite/common/history/history.xul, * line 129 -- oncommand="return SortInNewDirection('natural');"/> * line 135 -- oncommand="return SortInNewDirection('ascending');"/> * line 139 -- oncommand="return SortInNewDirection('descending');"/> ] <http://mxr.mozilla.org/seamonkey/search?string=%2FnsTreeSorting.js> [ /nsTreeSorting.js /xpfe/global/jar.mn, * line 20 -- content/global/nsTreeSorting.js (resources/content/nsTreeSorting.js) /suite/common/history/historyTreeOverlay.xul, * line 60 -- <script type="application/x-javascript" src="chrome://global/content/nsTreeSorting.js"/> ] I verified that the Windows zip nightly does not include <nsTreeSorting.js> ! Missing file after Xpfe to Toolkit migration ?

In toolkit 1.8, nsTreeSorting.js was in /toolkit/obsolete/content/nsTreeSorting.js, in toolkit 1.9 nsTreeSorting.js was CVS removed. Suggestion: 1. CVS copy /xpfe/global/resources/content/nsTreeSorting.js to /suite/common/history/ 2. Update /suite/common/jar.mn 3. Update the relevant line in historyTreeOverlay.xul

Per <http://mxr.mozilla.org/seamonkey/search?string=RefreshSort&case=on&tree=seamonkey> {{ /xpfe/global/resources/content/nsTreeSorting.js, * line 45 -- function RefreshSort() }}

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008040501 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4) Per comment 1 point 2 and 3. NB: I manually copied the file into my installed application, but I didn't/can't actually test the <jar.mn> part.

(In reply to comment #0) > Missing file after Xpfe to Toolkit migration ? [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/20070515 SeaMonkey/1.5a] (nightly) (W2Ksp4) <nsTreeSorting.js> was present in <...\chrome\toolkit.jar, \content\global\>. ***** (In reply to comment #1) > Suggestion: Thanks !

Comment on attachment 313876 [details] [diff] [review] (Av1) <nsTreeSorting.js> unused |function RefreshSort()| removal NB: I will file the "cvs copy" bug, after this patch checkin...

In #seamonkey Neil suggests that we just copy the required functions into history.js; based on some grepping I see that all the following functions are needed: SortInNewDirection() SortColumn() SortColumnElement() find_sort_column() find_sort_direction() update_sort_menuitems() enable_sort_menuitems() fillViewMenu() fillViewMenu() Which, er, is actually all the functions in nsTreeSorting.js except for RefreshSort(). Neil, your opinion please?

(In reply to comment #6) > fillViewMenu() > fillViewMenu() We definitely don't need to programmatically fill the view menu, since we already know what all of our columns are, so we can simply specify them in XUL. I think some of the other methods may be obsolete too, for instance find_sort_column() may be obsoleted by tree.columns.getSortedColumn().

Given that someone will be along shortly (well we can hope can't we?) to rewrite all this for Places, it it worthwhile to optimize the current code?

(In reply to comment #6) > Which, er, is actually all the functions in nsTreeSorting.js except for > RefreshSort(). That's what I found with MXR too: hence my Av1 patch. (In reply to comment #7) > I think some of the other methods may be obsolete too, for instance > find_sort_column() may be obsoleted by tree.columns.getSortedColumn(). |getSortedColumn()| is used once only: <http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/components/places/content/treeView.js&rev=1.60&mark=773,776#765> I made a few attempts to use |getSortedColumn()| in our code, but failed :-/ (In reply to comment #8) > Given that someone will be along shortly (well we can hope can't we?) to > rewrite all this for Places, it it worthwhile to optimize the current code? I would concur: make it work again here, then improve it in a followup bug.

(In reply to comment #8) > Given that someone will be along shortly (well we can hope can't we?) to > rewrite all this for Places, it it worthwhile to optimize the current code? Assuming we'll still have a tree-based History window, then we'll still need tree-based sorting for it...

Comment on attachment 313876 [details] [diff] [review] (Av1) <nsTreeSorting.js> unused |function RefreshSort()| removal >Index: nsTreeSorting.js >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/global/resources/content/nsTreeSorting.js,v The patch's objective as such is basically right, just the file is basically the wrong one. We should first have a nsTreeSorting.js under /suite and then patch that.

Comment on attachment 313877 [details] [diff] [review] (Bv1) Package file, update script src (In reply to comment #5) > NB: I will file the "cvs copy" bug, after this patch checkin... This might not be necessary, we're discussing a non-CVS-copy file move/fork policy atm.

Should we rename the file as well? historyTreeSorting.js perhaps?

Sounds reasonable.

poke, poke. Did we decide on a plan here? copy/rename the file along with something like attachment 31876 [details] [diff] [review] and then clean it up?

(Neil wrote in Comment 13) > Should we rename the file as well? historyTreeSorting.js perhaps? 1. "history" in historyTreeSorting.js is redundant since the file will be in chrome://communicator/content/_history_/ anyway. 2. K.I.S.S. (Andrew wrote in Comment 15) > Did we decide on a plan here? Dunno. I'll ask Neil for an sr anyway.

Comment on attachment 313877 [details] [diff] [review] (Bv1) Package file, update script src Let's get this checked in, any clean up should be moved to a follow-up bug to keep things focused.

Comment on attachment 313877 [details] [diff] [review] (Bv1) Package file, update script src OK, but don't forget to file the cleanup bug.

(In reply to comment #12) > (From update of attachment 313877 [details] [diff] [review]) > (In reply to comment #5) > > NB: I will file the "cvs copy" bug, after this patch checkin... > > This might not be necessary, we're discussing a non-CVS-copy file move/fork > policy atm. First/Now, I need someone to copy/move the file.

(In reply to Comment 18) > OK, but don't forget to file the cleanup bug. Submitted Bug 433707, but I can never remember in which direction the depends/blocks works. (In reply to Comment 19) > First/Now, I need someone to copy/move the file. 1. I've changed my mind, it should be "historyTreeSorting.js" to maintain consistency with the naming of the other files in that directory. 2. If we assume the "no-history" rule, just treat it as a new file. KaiRo? Need an executive decision.

Once we have comm-central, the file move is a non-issue. If you want to get the move done in CVS/1.9.0 before that, we need a CVS copy file for the target /suite/common/history/historyTreeSorting.js.

*"Restore" file which was removed by http://hg.mozilla.org/mozilla-central/rev/d98ddf4499cc and "move" it to its new place. *Merge Av1 and Bv1 patches too. *** I'd like to get |approval‑seamonkey2.0a1| too...

Comment on attachment 340052 [details] [diff] [review] (Cv1) all in one [Checkin: Comment 23] The file removal in XPFE was rather unfortunate, since this actually kills our VCS history, so please a respective comment about the content's heritage + * ***** END LICENSE BLOCK ***** */ + -> here ! + +// utility routines for sorting r=me with that

Comment on attachment 340052 [details] [diff] [review] (Cv1) all in one [Checkin: Comment 23] http://hg.mozilla.org/comm-central/rev/7450ecf7d520

(In reply to comment #23) > (From update of attachment 340052 [details] [diff] [review]) Per our irc: > The file removal in XPFE was rather unfortunate, since this actually kills our > VCS history, so please a respective comment about the content's heritage That's what I thought at first, but, actually, only bad it did was make me need to dig around to find out where I could restore this file from :-/ Regarding history which we wanted to keep (by a file move), that wasn't possible across repositories (m-c -> c-c); "fortunately", there was no new history in m-c :-) > -> here ! > r=me with that http://hg.mozilla.org/comm-central/rev/32f07da4a337

> Regarding history which we wanted to keep (by a file move), > that wasn't possible across repositories (m-c -> c-c); <http://www.selenic.com/mercurial/wiki/index.cgi/ConvertExtension> hg convert with the --filemap (with the include and rename directives) to pull one file (with history) into a temporary repository, then export the changesets into comm-central.