Last Comment Bug 371629 - Star header will not un-highlight
: Star header will not un-highlight
: regression, verified1.8.1.3
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: 2.0
: PowerPC Mac OS X
: -- normal (vote)
: Thunderbird2.0
Assigned To: Nobody; OK to take it and work on it
Depends on: 372261
Blocks: 256688
  Show dependency treegraph
Reported: 2007-02-25 09:26 PST by Sherman Dickman
Modified: 2009-08-12 07:59 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

branch only patch (3.16 KB, patch)
2007-02-27 17:31 PST, Scott MacGregor
neil: review+
Details | Diff | Splinter Review

Description Sherman Dickman 2007-02-25 09:26:42 PST
Once I click the star header to sort messages by their star status, that header will not un-highlight once I click on another header.  Thus, I'll have two headers highlighted, one always being the star header.
Comment 1 Sherman Dickman 2007-02-25 09:27:14 PST
Using the following:
version 2 beta 2 (20070223)
Comment 2 Scott MacGregor 2007-02-26 16:55:04 PST
this might be a bug in the pinstripe theme. I can't reproduce this on windows. on the mac works ok so it is a regression.
Comment 3 Scott MacGregor 2007-02-26 17:26:38 PST
Hey Neil, I'm seeing some weirdness with tree widgets on the 1.8 branch and the trunk. Have you seen anything like this before?

* The sortDirection attribute is not getting removed from treecol-icons when the sort order moves to another column. I do see the sortDirection attribute get removed for non icon based tree-cols. DOM Inspector for Windows and mac both show the attribute getting left behind.

* On the Mac, when a treecol-icon has the sortDirection attribute set on it, I see a sort arrow getting drawn underneath the treecol icon which shouldn't be there. Windows doesn't suffer this problem.
Comment 4 Neil Deakin 2007-02-26 18:58:59 PST
Which tree is this? If it's the thread pane, it has it's own implementation of handling sort indicators. (UpdateSortIndicators in threadPane.js) so there's probably a bug somewhere there.

Comment 5 Scott MacGregor 2007-02-26 23:46:15 PST
(In reply to comment #4)
> Which tree is this? If it's the thread pane, it has it's own implementation of
> handling sort indicators. (UpdateSortIndicators in threadPane.js) so there's
> probably a bug somewhere there.

Ah, right you are Neil. It looks like the bug is in UpdateSortIndicators  for the issue Sherman is seeing. I've got a fix for that.

The 2nd issue with the sort arrow getting drawn underneath the tree col icon, I'll spin that off into a separate bug. 
Comment 6 Scott MacGregor 2007-02-27 17:31:28 PST
Created attachment 256736 [details] [diff] [review]
branch only patch

UpdateSortIndicators scared me :)

This patch is for the branch only and it properly removes the sort indicator from all columns instead of just removing it from the columns to the right of the column we are going to sort by.

I have another patch for the trunk which does a bunch of cleanup on this method to make it a bit less scarier, but I don't want to put that on the branch.
Comment 7 2007-02-28 03:43:24 PST
This is a regression from bug 256688.
Comment 8 2007-02-28 04:11:30 PST
I can't actually reproduce the bug as described; the bug only applies if group by sort is in effect and changing the sort order always seems to turn that off.
Comment 9 2007-02-28 04:15:37 PST
Comment on attachment 256736 [details] [diff] [review]
branch only patch

>+  for (var i=0; i < treeColumns.length; i++)
Nit: spacing around i = 0

>+    treeColumns.item(i).removeAttribute('sortDirection');
Nit: treeColumns[i] should work.

But I'd still like to know how to reproduce this ;-)
Comment 10 Scott MacGregor 2007-02-28 23:31:00 PST
Ok, this is fixed on the branch with Neil's review comments. I'm leaving the bug open until I have time to finish up my patch that cleans up this routine.
Comment 11 Robert Kaiser 2007-03-01 06:35:25 PST
Apparently this checkin broke SeaMonkey mailnews on branch. When I pulled a current tree, thread (and msg) pane stayed empty and mouse cursor stayed in busy state in the mail window. After backing out those changes to mailnews/base/resources/content/threadPane.* it works again.
Comment 12 Scott MacGregor 2007-03-01 08:13:49 PST
I suck. Checking in a fix right now.
Comment 13 Scott MacGregor 2007-03-01 08:21:21 PST
Fix checked in sorry for the mess guys. 
Comment 14 Marcia Knous [:marcia - use ni] 2007-04-04 12:40:50 PDT
verified fixed on the 1.8 branch using the Tbird cand build, version (20070326). I verified using a PPC Mac running 10.3.9. I am able to switch back and forth between the star header and other headers with no issues. Adding branch keyword.
Comment 15 Magnus Melin 2008-08-25 13:50:12 PDT
Is it a problem on trunk? I don't see it on linux.
Comment 16 Wayne Mery (:wsmwk, NI for questions) 2009-08-12 07:59:05 PDT
does not fail on trunk, so closing FIXED

Note You need to log in before you can comment on or make changes to this bug.