Last Comment Bug 594053 - wrong order arrow in message list
: wrong order arrow in message list
Status: RESOLVED FIXED
: modern
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: unspecified
: x86 Windows XP
: -- normal (vote)
: seamonkey2.1b3
Assigned To: Edmund Wong (:ewong)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-07 09:46 PDT by tonda kavalec
Modified: 2011-02-06 18:50 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Screenshot of both themes, showing indicator change. (30.42 KB, image/png)
2010-09-07 10:42 PDT, therube
no flags Details
Switched the sort-asc.gif and sort-dsc.gif to reflect their actual direction. (827 bytes, patch)
2011-01-06 17:28 PST, Edmund Wong (:ewong)
neil: review-
Details | Diff | Splinter Review
Fixed the order of the arrows in the message list. (2.03 KB, patch)
2011-02-05 19:14 PST, Edmund Wong (:ewong)
no flags Details | Diff | Splinter Review
Fixed the order of the arrows in the message list. (v3) (2.83 KB, patch)
2011-02-06 07:16 PST, Edmund Wong (:ewong)
no flags Details | Diff | Splinter Review
Fixed the order of the arrows in the message list. (v4) (2.69 KB, patch)
2011-02-06 15:27 PST, Edmund Wong (:ewong)
neil: review+
Details | Diff | Splinter Review

Description tonda kavalec 2010-09-07 09:46:31 PDT
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100907 SeaMonkey/2.1b1pre
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100907 SeaMonkey/2.1b1pre

 the same in 2.0.6 and 2.1

Reproducible: Always

Steps to Reproduce:
1. click the sort arrow to sort messages per date in descendent order - newest on top oldest at the bottom.
2.
3.
Actual Results:  
sorting descendent, the direction of the arrow is to the top in modern theme. In default theme is the arrow show to the bottom V. the same in 2.0.6 and 2.1

Expected Results:  
while descending order arrow should go to the bottom like it does in default theme
Comment 1 therube 2010-09-07 10:41:05 PDT
Confirmed.

Just to clarify ...

The (Mail header column) sort direction arrow changes depending upon the theme (Modern or Default) used.

Underlying bug perhaps (or the Depends): Bug 93772 - The arrow indicating sort direction is reversed

> <RattyAway>	probably nobody remembered to update Modern.
Comment 2 therube 2010-09-07 10:42:07 PDT
Created attachment 472679 [details]
Screenshot of both themes, showing indicator change.
Comment 3 Edmund Wong (:ewong) 2011-01-06 06:52:05 PST
Taking this bug for a spin.
Comment 4 Edmund Wong (:ewong) 2011-01-06 17:28:58 PST
Created attachment 501880 [details] [diff] [review]
Switched the sort-asc.gif and sort-dsc.gif to reflect their actual direction.
Comment 5 Robert Kaiser 2011-01-07 11:52:22 PST
Comment on attachment 501880 [details] [diff] [review]
Switched the sort-asc.gif and sort-dsc.gif to reflect their actual direction.

Sorry, I'm not a good reviewer for this, let's let Neil do it instead.
Comment 6 neil@parkwaycc.co.uk 2011-01-07 12:04:50 PST
To add to the confusion, Linux currently agrees with Modern...
Comment 7 Edmund Wong (:ewong) 2011-01-13 17:35:22 PST
(In reply to comment #6)
> To add to the confusion, Linux currently agrees with Modern...

Perhaps I can spin a new bug to tackle this Linux wierdness?
Comment 8 neil@parkwaycc.co.uk 2011-01-18 06:26:35 PST
Comment on attachment 501880 [details] [diff] [review]
Switched the sort-asc.gif and sort-dsc.gif to reflect their actual direction.

So, the decision is that we don't want to change the arrows on Linux. There are a few potential ways of providing different sort arrows on different platforms:
1. Ask layout for a pseudoclass that can be used in CSS to reverse the arrows
2. Ask toolkit for an attribute that can be used by CSS to reverse the arrows
3. Create an OS-dependent skin package that reverses the arrows by platform
4. Package different versions of Modern on Windows and Mac (ugly, I know)
Comment 9 Philip Chee 2011-01-18 06:49:53 PST
3vil hack proposal:

hg add /suite/themes/modern/global/tree/sort-asc.gif b/suite/themes/modern/global/tree/reverse-sort-asc.gif
hg add /suite/themes/modern/global/tree/sort-asc.gif b/suite/themes/modern/global/tree/reverse-sort-dsc.gif

jar.mn
override chrome://global/skin/tree/sort-asc.gif chrome://global/skin/tree/reverse-sort-asc.gif os=Linux
override chrome://global/skin/tree/sort-dsc.gif chrome://global/skin/tree/reverse-sort-dsc.gif os=Linux

Note will obviously miss SunOS FreeBSD OpenBSD NetBSD AIX HP-UX DragonFly OSF1
See https://developer.mozilla.org/en/OS_TARGET

It might be better to do it the other way for only WINNT and DARWIN

override chrome://global/skin/tree/sort-asc.gif chrome://global/skin/tree/reverse-sort-asc.gif os=WINNT
override chrome://global/skin/tree/sort-dsc.gif chrome://global/skin/tree/reverse-sort-dsc.gif os=WINNT
override chrome://global/skin/tree/sort-asc.gif chrome://global/skin/tree/reverse-sort-asc.gif os=Darwin
override chrome://global/skin/tree/sort-dsc.gif chrome://global/skin/tree/reverse-sort-dsc.gif os=Darwin

Everything else is assumed to be some sort of *nix.
Question: what about a hypothetical qt build?
Comment 10 neil@parkwaycc.co.uk 2011-01-18 06:52:33 PST
Very evil, as it will affect all 3rd party themes using those chrome URLs.
Comment 11 Philip Chee 2011-01-18 06:52:54 PST
urk that should be:
hg add /suite/themes/modern/global/tree/sort-asc.gif
hg add /suite/themes/modern/global/tree/sort-asc.gif
Comment 12 Philip Chee 2011-01-18 06:54:17 PST
I mean /suite/themes/modern/jar.mn
Comment 13 Philip Chee 2011-01-18 06:55:08 PST
one more try:
urk that should be:
hg add /suite/themes/modern/global/tree/reverse-sort-asc.gif
hg add /suite/themes/modern/global/tree/reverse-sort-asc.gif
Comment 14 Philip Chee 2011-01-18 08:09:26 PST
Grr. According to Neil the override directive does not work in theme chrome.manifest.
Comment 15 jag (Peter Annema) 2011-01-18 08:25:19 PST
Do you need the override directive though?

Could we do something like this:

===== themes/modern/jar.mn =====
modern.jar:
...
  skin/modern/global/tree/sort-asc.gif (global/tree/sort-asc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/sort-dsc.gif)
...

modern.jar:
% skin global modern/1.0 %skin/modern/global/ os=WINNT
% skin global modern/1.0 %skin/modern/global/ os=OS2
% skin global modern/1.0 %skin/modern/global/ os=Darwin
  skin/modern/global/tree/sort-asc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/sort-asc.gif)
Comment 16 Philip Chee 2011-01-18 08:35:42 PST
This might work:

% skin global-platform modern/1.0 %skin/modern/global/
% skin global-platform modern/1.0 %skin/modern/global/unix/ os=Linux

tree.css:

.treecol-sortdirection[sortDirection="ascending"] {
  list-style-image: url("chrome://global-platform/skin/tree/sort-asc.gif");
}

.treecol-sortdirection[sortDirection="descending"] {
  list-style-image: url("chrome://global-platform/skin/tree/sort-dsc.gif");

ditto for listbox.css
Comment 17 Philip Chee 2011-01-18 08:40:36 PST
Plus:
jar.mn:
  skin/modern/global/tree/sort-asc.gif (global/tree/sort-asc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/tree/sort-asc.gif (global/tree/unix/sort-dsc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/unix/sort-asc.gif)
Comment 18 jag (Peter Annema) 2011-01-18 09:06:47 PST
In comment 15, those last two lines need a + of course. I think I still like that a bit better than comment 16 and 17 (also needs +es).

If we do end up going with comment 16/17, we could do this:

modern.jar:
% skin global-platform modern/1.0 %skin/modern/global/ os=WINNT
% skin global-platform modern/1.0 %skin/modern/global/ os=OS2
% skin global-platform modern/1.0 %skin/modern/global/ os=Darwin
% skin global-platform modern/1.0 %skin/modern/global/unix/
  skin/modern/global/tree/sort-asc.gif (global/tree/sort-asc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/tree/sort-asc.gif (global/tree/unix/sort-dsc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/unix/sort-asc.gif)

And depend on the files not getting overwritten for the Win/OS2/Mac case.
Comment 19 Philip Chee 2011-01-18 11:09:30 PST
>   skin/modern/global/tree/sort-asc.gif (global/tree/unix/sort-dsc.gif)
global/unix/tree/sort-dsc.gif no?
Comment 20 jag (Peter Annema) 2011-01-18 11:11:20 PST
Heh, yeah, I hadn't even noticed (copied them from comment 17).

But see the first part of comment 18 ;-)
Comment 21 Philip Chee 2011-01-18 19:16:04 PST
> But see the first part of comment 18 ;-)
But we aren't replacing existing files as far as I can see. sort-asc.gif and sort-dsc.gif will just live in two places each in the resulting .jar file.
Comment 22 Philip Chee 2011-01-18 19:19:10 PST
Bah just reread the docs at a saner time of the day
  skin/modern/global/tree/sort-asc.gif (global/tree/sort-asc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/unix/tree/sort-asc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/unix/tree/sort-dsc.gif (global/tree/sort-asc.gif)
Comment 23 jag (Peter Annema) 2011-01-18 19:48:39 PST
Ah. Heh, I totally misunderstood the meaning of the third arg of the "skin" directive (though now that I look at it, it couldn't possibly have meant what I thought it meant). Which also means that what I wrote in comment 18 is nonsense.

As much as I dislike the files being duplicated, I don't see a better alternative if we desire to ship the same modern.jar everywhere.

So would the following work?

modern.jar:
% skin global-platform modern/1.0 %skin/modern/global/      os=WINNT
% skin global-platform modern/1.0 %skin/modern/global/      os=OS2
% skin global-platform modern/1.0 %skin/modern/global/      os=Darwin
% skin global-platform modern/1.0 %skin/modern/global/unix/ os!=WINNT os!=OS2 os!=Darwin
  skin/modern/global/tree/sort-asc.gif (global/tree/sort-asc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/unix/tree/sort-asc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/unix/tree/sort-dsc.gif (global/tree/sort-asc.gif)
Comment 24 Edmund Wong (:ewong) 2011-02-01 02:07:43 PST
> modern.jar:
> % skin global-platform modern/1.0 %skin/modern/global/      os=WINNT
> % skin global-platform modern/1.0 %skin/modern/global/      os=OS2
> % skin global-platform modern/1.0 %skin/modern/global/      os=Darwin
> % skin global-platform modern/1.0 %skin/modern/global/unix/ os!=WINNT os!=OS2
> os!=Darwin
>   skin/modern/global/tree/sort-asc.gif (global/tree/sort-asc.gif)
>   skin/modern/global/tree/sort-dsc.gif (global/tree/sort-dsc.gif)
>   skin/modern/global/unix/tree/sort-asc.gif (global/tree/sort-dsc.gif)
>   skin/modern/global/unix/tree/sort-dsc.gif (global/tree/sort-asc.gif)

Does this include the fact that I need to change the actual gif files?
Right now, sort-asc has a descending arrow, and sort-dsc has an ascending
arrow.
Comment 25 Philip Chee 2011-02-01 02:27:22 PST
You could avoid changing the actual gif files if you do the right mappings in the jar.mn file.

And of course you need to change the css:

.treecol-sortdirection[sortDirection="ascending"] {
  list-style-image: url("chrome://global-platform/skin/tree/sort-asc.gif");
}
Comment 26 jag (Peter Annema) 2011-02-01 04:25:48 PST
Yeah, so you'd actually want:

  skin/modern/global/tree/sort-asc.gif (global/tree/sort-dsc.gif)
  skin/modern/global/tree/sort-dsc.gif (global/tree/sort-asc.gif)
  skin/modern/global/unix/tree/sort-asc.gif (global/tree/sort-asc.gif)
  skin/modern/global/unix/tree/sort-dsc.gif (global/tree/sort-dsc.gif)

since the modern theme uses the Unix concept of ascending/descending arrows for those file names.
Comment 27 Edmund Wong (:ewong) 2011-02-05 18:49:24 PST
(In reply to comment #25)
> You could avoid changing the actual gif files if you do the right mappings in
> the jar.mn file.
> 
> And of course you need to change the css:
> 
> .treecol-sortdirection[sortDirection="ascending"] {
>   list-style-image: url("chrome://global-platform/skin/tree/sort-asc.gif");
> }

this doesn't work for me.  Keeping it as //global/ works. (This is with
jag's suggested % skin ... lines.)
Comment 28 Edmund Wong (:ewong) 2011-02-05 19:14:19 PST
Created attachment 510083 [details] [diff] [review]
Fixed the order of the arrows in the message list.
Comment 29 neil@parkwaycc.co.uk 2011-02-06 04:15:10 PST
This is not how you use global-platform. It actually automatically adds mac, unix or win to your folder path for you. So all you have to do is to create three sets of entries in jar.mn for mac/tree, unix/tree and win/tree for the files themselves and one chrome registration entry. (Don't forget tree.css!)
Comment 30 Edmund Wong (:ewong) 2011-02-06 07:16:28 PST
Created attachment 510117 [details] [diff] [review]
Fixed the order of the arrows in the message list. (v3)
Comment 31 neil@parkwaycc.co.uk 2011-02-06 08:51:41 PST
Comment on attachment 510117 [details] [diff] [review]
Fixed the order of the arrows in the message list. (v3)

>+% skin global-platform modern/1.0 %skin/modern/global/
> % skin global modern/1.0 %skin/modern/global/
Nit: please put global-platform after global (space sorts before hyphen).

>+  skin/modern/global/os2/tree/sort-asc.gif                         (global/tree/sort-dsc.gif)
>+  skin/modern/global/os2/tree/sort-dsc.gif                         (global/tree/sort-asc.gif)
There's no "os2" platform. OS/2 gets lumped in with Windows.

>+  skin/modern/global/darwin/tree/sort-asc.gif                      (global/tree/sort-dsc.gif)
>+  skin/modern/global/darwin/tree/sort-dsc.gif                      (global/tree/sort-asc.gif)
The Mac platform is called "mac", strangely enough ;-)

http://mxr.mozilla.org/comm-central/source/mozilla/chrome/src/nsChromeRegistry.cpp?mark=334,336,338#332
Comment 32 Edmund Wong (:ewong) 2011-02-06 15:27:31 PST
Created attachment 510181 [details] [diff] [review]
Fixed the order of the arrows in the message list. (v4)
Comment 33 Justin Wood (:Callek) 2011-02-06 18:50:52 PST
http://hg.mozilla.org/comm-central/rev/892088e4ead3

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