Closed Bug 4731 Opened 26 years ago Closed 26 years ago

Sort order strange

Categories

(MailNews Core :: Backend, defect, P3)

x86
Windows 95
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: esther, Assigned: mozilla)

References

Details

(Whiteboard: DEPEMD - Intl)

Seamonkey 1999040710 win32 April 7 build Hardware: Win95 1. Launch apprunner 2. Open Messenger via Task menu 3. Select a Folder that has several messages 4. Click on the the Sender column. The sort happens, but the order is not alphabetical or even recognizable. (If you can't see the problem using one of your folders, go to this test account (qatest04) and sort the Inbox.)
Assignee: phil → putterman
Target Milestone: M5
virtual phil --> re-assigning to scott putterman. scott this is the problem we were talking about at lunch where rdf is using strcmp against the collation keys we are giving them. I'm assuming you'll eventually pass this on to rdf (Robert Churchill?).
Assignee: putterman → rjc
Robert, This is what I've written you about in the messages I've been sending. From the message from Naoki: I fount that the generated keys are compared by nsString.Compare whilch uses strcasecmp. Generated keys are supposed to be compared by a comparison funcion in the collation interface (strcmp is very bad because it not only compares wrong but it will lower case the generated binary key). The function below need to call the CompareSortKey in nsICollation. I know that generated key for Linux may contain zero, that's probably why Linux looks bad. Anyway, would you file a bug for this and assign to whoever owning the code.
*** Bug 4735 has been marked as a duplicate of this bug. ***
*** Bug 4898 has been marked as a duplicate of this bug. ***
Robert, any ideas on how we can solve this?
Linux build 04-20-08 has the sorting problem. (NT4 build 04-20-10 doesn't have the sorting problem.) I have 52 messages in the Inbox, sorted by Subject and their order is: - Return Receipt - Earth.gif - Wednesday - Ivy.gif - Friday - Blank messages (which have Parsing Errors) I sorted on Subject again and it just reversed the order: - Blank messages (which have Parsing Errors) - Friday - Ivy.gif - Wednesday - Earth.gif - Return Receipt
It's reversing the order because when you click on it once it sorts it ascending and then when you click on it again it sorts descending. So that appears to be working (even if the sorting isn't).
Assignee: rjc → putterman
I believe the real problem isn't with the sorting code but with the XUL and JavaScript in mozilla\mailnews\ui\messenger\resources\threadPane.xul Take a look at mozilla\rdf\resources\sidebar.xul (near the end of the file) and note how every <XUL:TREECELL> node contains a couple of <XUL:OBSERVES> nodes which observe the "sortActive" and "sortDirection" properties of the respective column elements. I suspect that threadPane.xul needs to do the same thing for sorting to work properly. :^) I'm reassigning this back to you, Scott, to try it out.
I can look into that, but I'm pretty sure sorting is taking place given that on Windows sorting pretty much works correctly, and your code is calling my datasource with "property?=sort". I think there is a problem with the fact that we are returning collation keys and you are using strcmp not CompareSortKey. I think we need the ability for the datasource to provide the compare function so that those of us who use collation keys can compare them correctly and those datasource that don't use collation keys can just use strcmp.
Target Milestone: M5 → M6
I need to verify that it's not my fault. I'm pretty sure it's not. Anyway, it's doubtful this will get fixed for M5. I'm moving it to M6.
Assignee: putterman → rjc
I'm reassigning this back to rjc. I'm pretty sure there's a problem with the current RDF sorting architecture or I'm not sure how to adapt it to our needs. First of all, I don't think the lack of observers is making any difference. I'm not sure what the observers are for, but I'd imagine they'd be for making the UI show up correctly in the column headers. Second, I went to rjc's sort code and changed his compare to use the sort key compare in the intl code. Sure enough message sorting worked just fine, much better than when using the current code. The problem is that we need to use the collation sort key compare. But we shouldn't have to require everyone who wants to play in RDF to use collation keys. We also shouldn't prevent those of us who want to from using collation keys. So either we need to add a way to tell RDF that we're using collation keys or RDF has to allow us to specify our compare function so that we can use collation keys if we want to.
Status: NEW → ASSIGNED
QA Contact: 4080 → 4104
Note: I'm slowly adding support for collation keys. It might fully make it into M6 and it might not.
Assignee: rjc → putterman
Status: ASSIGNED → NEW
Scott, the XUL sort code has been changed to now first ask for a column attribute with "?collation=true" appended to it. If you change the mail/news code to provide an answer for this request, and two collation keys are to be sorted, the collation service will be used instead of a straight string comparison. I'm reassigning this back to you... let me know if you have any problems.
Assignee: putterman → rjc
I have changes for mail that I will check in. But this still doesn't work. In XULSortServiceImpl::GetNodeValue(nsIContent *node1, nsIRDFResource *sortProperty, sortPtr sortInfo, nsString &cellVal1, PRBool &isCollationKey) nsCOMPtr<nsIRDFResource> res1 = do_QueryInterface(dom1); // Note: don't check for res1 QI failure here. It only succeeds for RDF nodes, // but for XUL nodes it will failure; in the failure case, the code below gets // the cell's text value straight from the DOM Is always returning res1 = null. Therefore I never get called with ?collation=true. Reassigning back to rjc.
I changed the line I mentioned above to nsCOMPtr<nsIRDFResource> res1; rv = dom1->GetResource(getter_AddRefs(res1)); if(NS_FAILED(rv)) res1 = null_nsCOMPtr(); and everything worked great. I got the same sort order as 4.5. Chris had mentioned not wanting us to use GetResource from the dom node, but since we already know it's a DOMXULNode, it should be ok. I'm not sure setting res1 to null is necessary, but I was trying to duplicate the other code. Anyway, feel free to add this or do it some other way. When the tree opens, I'll check in my changes, and if you can check in a fix for this, this should work.
Status: NEW → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
Scott, I checked in your change. Thanks!
Whiteboard: DEPEMD - Intl
Status: RESOLVED → VERIFIED
RE: Win32 on Win_Nt 4.0 and win 95 In the single account, sort order does not work the first time. But it works the second time and thereafter. In a multiple account, sort order also does not work the first time. When I switch from one account to another, it also shows the first sort does not work. Since the second sort sorts alphabetically, I am verifying this bug and open a new bug for the new problem.
This bug is fixed. The problems that Fenella is seeing is due to 2 things. One is a duplicate of 6670. Add further sorting to the list of things that get messed up when we sort. The other is that we are sorting by Author, not by sender name even though we are displaying the sender's name. So the sort order may not look correct, even though it is, the first time. Amusingly enough, even though it's the 2nd time onward that looks ok, it's really the 2nd time onward that is acting incorrectly.
Status: VERIFIED → REOPENED
I am re-opening this bug pending Scott P. 's finding. What we (Scott and I) is: The first sort was sorted by the From field, not the text field as it supposed to in the Sender column. He said it could be the same bug. Scott told me not to write a new bug report.
Status: REOPENED → RESOLVED
Closed: 26 years ago26 years ago
This one is fixed. I added more info to 6670 discussing this.
Status: RESOLVED → VERIFIED
I have written a new bug 7028 to address the sort by From field issue. mark this one verified.
Blocks: 7228
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.