Last Comment Bug 840003 - Icons for special IMAP folder (Inbox, trash, ...) are not shown when folder is shared
: Icons for special IMAP folder (Inbox, trash, ...) are not shown when folder ...
Status: VERIFIED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.19
Assigned To: Tony Mechelynck [:tonymec]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-11 04:19 PST by Rémi KUSIAK
Modified: 2013-03-29 12:09 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
verified


Attachments
imap special with generic icon (849 bytes, image/png)
2013-02-11 04:19 PST, Rémi KUSIAK
no flags Details
imap special with special icon (1.66 KB, image/png)
2013-02-11 04:28 PST, Rémi KUSIAK
no flags Details
proposed patch (4.11 KB, patch)
2013-02-25 00:13 PST, Tony Mechelynck [:tonymec]
no flags Details | Diff | Splinter Review
patch v0.1 (don't override new mail) (3.77 KB, patch)
2013-03-06 08:14 PST, Tony Mechelynck [:tonymec]
neil: review+
mnyromyr: superreview+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta-
Details | Diff | Splinter Review

Description Rémi KUSIAK 2013-02-11 04:19:12 PST
Created attachment 712395 [details]
imap special with generic icon

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:18.0) Gecko/20100101 Firefox/18.0 SeaMonkey/2.15.2
Build ID: 20130203000651

Steps to reproduce:

Setup imap account.


Actual results:

Special folder (Inbox trash, Junk, ...)  have the generic shared folder icon when these folders are shared (they can't be unshared). See attachement.


Expected results:

Special folders should have special icons.
Comment 1 Rémi KUSIAK 2013-02-11 04:28:24 PST
Created attachment 712397 [details]
imap special with special icon
Comment 2 Rémi KUSIAK 2013-02-11 04:35:21 PST
workaround: removed the folder-share.gif definition from folderPane.css in modern@themes.mozilla.org.xpi to have better look. See attachement 2
Comment 3 Philip Chee 2013-02-23 06:36:20 PST
Does this problem also occur in the Default (Classic) theme? Go View->Apply Theme.
Comment 4 Rémi KUSIAK 2013-02-23 10:38:30 PST
Yes it's the same for default theme (but the workaround can't be done as there is no xpi for the default theme)

(The IMAP server is Zimbra from french ISP free.fr )
Comment 5 Tony Mechelynck [:tonymec] 2013-02-24 23:20:17 PST
The rule

/* ...... Shared ..... */

.icon-holder[type="folder"][ImapShared="true"],
treechildren::-moz-tree-image(folderNameCol, imapShared-true) {
  list-style-image: url("chrome://messenger/skin/icons/folder-share.gif");
}

(APPFOLDER/extensions/modern@themes.mozilla.org.xpi::chrome/modern/skin/modern/messenger/folderPane.css lines 103-108) is after most of the other rules, so the "shared folder" icon overrides the other ones by virtue of the CSS "last one wins" principle.

I believe that this could be corrected for Modern by moving that rule upward, to somewhere above /* ..... Inbox ..... */

For the default theme, there are supposedly similar CSS rules somewhere, but not in {972ce4c6-7e08-4474-a285-3208198ce6fd}.xpi which seems to be hardly more than a placeholder.

However, third party themes seem to have followed the example of the buil-in themes: for instance, my favourite EarlyBlue theme also has

.icon-holder[type="folder"][ImapShared="true"],
treechildren::-moz-tree-image(folderNameCol, imapShared-true) {
  -moz-padding-end: 2px;
  list-style-image: url("chrome://messenger/skin/icons/folder-share.gif");
}

after the rules for most other kinds of folders (at PROFILENAME/extensions/EarlyBlue@kairo.at.xpi::messenger/folderPane.css lines 107-111) and of course modifying the order of lines in all third-party themes lies beyond the power of Mozilla developers.

I think that fixing this bug would be relatively easy for each theme in particular, but terribly hard to do for all themes (including all third-party themes) at approximately the same time.
Comment 6 Tony Mechelynck [:tonymec] 2013-02-24 23:49:25 PST
For the default theme, the offending CSS rule seems to be

.icon-holder[type="folder"][ImapShared="true"],
treechildren::-moz-tree-image(folderNameCol, imapShared-true) {
  list-style-image: url("chrome://messenger/skin/icons/folder-share.png");
}

at omni.ja/chrome/classic/skin/classic/messenger/folderPane.css lines 107-110.
Comment 7 Tony Mechelynck [:tonymec] 2013-02-25 00:13:57 PST
Created attachment 717761 [details] [diff] [review]
proposed patch

Neil, Mnyromyr: I'm sitting on the fence about whether or not this bug merits fixing. Feel free to set RESOLVED WONTFIX if it doesn't.
Comment 8 Tony Mechelynck [:tonymec] 2013-02-25 00:17:26 PST
Oops: Neil: Looks like I forgot to CC you when setting "r?", see comment #7.
Comment 9 neil@parkwaycc.co.uk 2013-03-03 07:41:27 PST
(Sorry for taking a week to get back to you. I don't think I have access to a server that supports shared folders any more and even when I did it didn't support shared special folders, although I suppose I could have designated a shared folder as special folder.)

I suppose we need to decide exactly how (un)important it is to know that a folder is shared, so that the rule is ordered correctly. I have a sneaky feeling that it's less important than everything else, including whether the folder has any new messages (is it possible for shared folders to have new messages?), in which case the rule should be the second image rule in the file.
Comment 10 Tony Mechelynck [:tonymec] 2013-03-06 08:14:03 PST
Created attachment 721736 [details] [diff] [review]
patch v0.1 (don't override new mail)

shared-folder rule moved a little higher in the theme to avoid overriding new mail icon
Comment 11 Tony Mechelynck [:tonymec] 2013-03-06 08:25:50 PST
bug 848292 (the corresponding Tb bug) _is_ related but both bugs can be fixed (or WONTFIXed) in any order.
Comment 12 Tony Mechelynck [:tonymec] 2013-03-07 19:42:58 PST
Could someone (mcsmurf?) please request a W32 try-build with the patch so that the reporter could test it?
Comment 13 Tony Mechelynck [:tonymec] 2013-03-07 20:48:53 PST
I've just posted a message on mozilla.dev.apps.seamonkey (and mozilla.dev.apps.thunderbird for bug 848292) in the hope that the authors of existing themes will see it and take the necessary action. I'm also ("just in case") adding to the CC list of this bug one such author "that I know of".
Comment 14 Frank Wein [:mcsmurf] 2013-03-09 06:11:47 PST
(In reply to Tony Mechelynck [:tonymec] from comment #12)
> Could someone (mcsmurf?) please request a W32 try-build with the patch so
> that the reporter could test it?

No need for a try build I think, Rémi can test the official nightly build as soon as the patch has been checked in (SeaMonkey try builds are not really trivial).
Comment 15 Robert Kaiser 2013-03-09 06:37:37 PST
(In reply to Tony Mechelynck [:tonymec] from comment #13)
> I'm also ("just
> in case") adding to the CC list of this bug one such author "that I know of".

Thanks - though I pick all those things up anyhow once they go to beta, as I always sync my themes to that channel and try toship that shortly before or around release of final. :)
Comment 16 Philip Chee 2013-03-16 07:01:28 PDT
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/db4428f24ef9
Comment 17 Tony Mechelynck [:tonymec] 2013-03-16 23:05:29 PDT
Rémi:

The patch has now been brought into the SeaMonkey "bleeding-edge" source tree, but, alas, at the moment Windows builds fail one after another due to some other bug(s), probably including bug 849609 and bug 784848. At least one successful build with the fix have been done for each of Linux32, Linux64 and Mac but IIUC you cannot use them.

Once the Windows builders start going again, W32 builds with the fix should appear at:
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-win32/?C=M;O=D (hourlies: each build has its own subdirectory)
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/latest-comm-central-trunk/?C=M;O=D (nightlies: these are the "latest" nightlies for each platform, older nightlies are found elsewhere).
"Hourlies" are compiled hwenever the source changes, "nightlies" once every 24 hours. In both cases the ?C=M;O=D at the end of the URL means "sorted latest-first"; check the date before you download, "interesting" timestamps are those later than comment #16.

These are "bleeding-edge" development builds; they are far from ready for prime time. Don't use them at work, and it is recommended to install them at a nonstandard custom location to avoid erasing your "production" executable (which ought to be SeaMonkey 2.16.2 by now, see http://www.seamonkey-project.org/ ), and use them with a new profile to avoid any risk of damage to your usual profile (see http://kb.mozillazine.org/Profile_Manager ). Also avoid removing mail from the server when read (but IIUC, IMAP mail usually remains on the server).

I would be grateful if you could test one of those W32 SeaMonkey 2.19a1 executables, once they start coming again, and report the results here. But please be extra-super-hyper-careful using them, they are not, repeat not, supposed to be anything near stable.
Comment 18 Tony Mechelynck [:tonymec] 2013-03-19 21:41:38 PDT
In reply to comment #17:

Win32 hourlies have resumed; I don't see any nightly yet but I hope there will be one in a few hours. If you want to install an hourly, find the latest subdirectory containing more than just a .txt.gz (which, if alone, would be the log of a failed build). Use the .zip or the .installer.exe, and use a non-default install directory to avoid clobbering your usual build.

Rémi, I don't wish you good luck because most French people think it brings bad luck. Je te souhaite un énorme tas de merde, comme ce que laisse un éléphant.
Comment 19 Rémi KUSIAK 2013-03-20 15:30:57 PDT
Patch is working well for Modern and Default themes: now I have the same special icons on special folders for IMAP accounts with or without shared folders.
Comment 20 Tony Mechelynck [:tonymec] 2013-03-21 14:17:22 PDT
Setting VERIFIED on account of comment #19 (patch is platform-agnostic CSS-only and the reporter testifies that it works on Windows) -- Thanks Rémi!
Comment 21 Tony Mechelynck [:tonymec] 2013-03-21 14:29:45 PDT
Comment on attachment 721736 [details] [diff] [review]
patch v0.1 (don't override new mail)

[Approval Request Comment]
Regression caused by (bug #): no bug (old error had gone unnoticed)
User impact if declined: no special icons if sharing special folders on IMAP
Testing completed (on m-c, etc.): yes, m-c, see comment #19 and #20
Risk to taking this patch (and alternatives if risky): minimal. Patch consists only of a reordering of existing CSS rules in both built-in themes.
String changes made by this patch: none
Comment 22 Tony Mechelynck [:tonymec] 2013-03-21 15:23:21 PDT
P.S. Patch applies cleanly on aurora with not even an offset. I don't have a comm-beta clone but I expect low bitrot rate in these two source files.
Comment 23 Justin Wood (:Callek) 2013-03-28 20:30:55 PDT
Comment on attachment 721736 [details] [diff] [review]
patch v0.1 (don't override new mail)

Review of attachment 721736 [details] [diff] [review]:
-----------------------------------------------------------------

a- for beta since we already spun our last beta this cycle
Comment 24 Tony Mechelynck [:tonymec] 2013-03-28 20:33:03 PDT
Thanks Justin!
Comment 25 Ryan VanderMeulen [:RyanVM] 2013-03-29 11:54:19 PDT
https://hg.mozilla.org/releases/comm-aurora/rev/5705670f1e11
Comment 26 Tony Mechelynck [:tonymec] 2013-03-29 12:09:01 PDT
Rémi: The fix has now been ported to the source tree for SeaMonkey 2.18a2 Aurora, just in time for it to be included (in a few days) in the first 2.18 beta, and about six weeks later in the 2.18 release. :-)

The current 2.18a2 nightly (built before comment #25) hasn't yet got the fix, but the next ones (dated 30 March and later) will have it.

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