147448 - Mailnews theme: duplicate images / css rules

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Håkan Waara]

In the Modern theme's CSS, there are a lot of rules for watched/ignored threads
and images that are duplicates. We could optimize this a bit.

For example:

treechildren:-moz-tree-image(news, threadCol, container, hasUnread, watch, open) {
  list-style-image: url("chrome://messenger/skin/icons/thread-new-open-eye.gif");
}

uses the exact same image as:

treechildren:-moz-tree-image(news, threadCol, container, hasUnread, watch) {
  list-style-image: url("chrome://messenger/skin/icons/thread-new-closed-eye.gif");
}

So basically we make no distinction between open and closed threads in the
watched/ignored icon, which makes sense, so let's remove the bloat.

BTW, I haven't investigated if Classic is optimizable too.

Comment 1

[Håkan Waara]

"But wait, there's more!"  Even ancient theme rules like these are duplicate:

treechildren:-moz-tree-image(threadCol, container) {
  list-style-image: url("chrome://messenger/skin/icons/thread-closed.gif");
}

treechildren:-moz-tree-image(threadCol, container, open) {
  list-style-image: url("chrome://messenger/skin/icons/thread-open.gif");
}

We could have just one "thread.gif" instead of two duplicate with different names.

Comment 2

[Håkan Waara]

Taking to investigate if I can just remove the redundant images and rules (maybe
it will speed us up, who knows, it can't hurt).

Comment 3

[Håkan Waara]

Attachment: Patch to Modern

OK, I tested it, and as expected, the rules (and images) are just plain
redundant. Luckily, this is only for threads, and not for any other aspect of
the mail theme (for modern).

Comment 4

[Håkan Waara]

I'll investigate Classic tomorrow.  Feel free to review the patch, so we can
unload the thread pane a bunch of CSS rules and images. :-)

Comment 5

[Håkan Waara]

Yep. The duplication is only in the Modern theme; the Classic theme actually
distinguishes between e.g., closed and open threads.

Comment 6

[Jan Varga [:janv]]

Comment on attachment 85201 [details] [diff] [review]
Patch to Modern

r=varga

Comment 7

[Håkan Waara]

Attachment: Additional patch to jar.mn

Additional patch needed to remove these files from Modern's jar.mn also.

Comment 8

[Jan Varga [:janv]]

Comment on attachment 85262 [details] [diff] [review]
Additional patch to jar.mn

r=varga

Comment 9

[(not reading, please use [e-mail] instead)]

those images may be place holders for images that we don't have yet.

meaning, the rules may be correct, we just didn't have images yet so we copied
existing images.

Comment 10

[Håkan Waara]

When I asked hewitt about this on IRC he said it sounds like a bug...

Comment 11

[Håkan Waara]

Sorry, I mean on AIM.

Comment 12

[(not reading, please use [e-mail] instead)]

> So basically we make no distinction between open and closed threads in the
> watched/ignored icon, which makes sense, so let's remove the bloat.

now I get it.  sr=sspitzer on both patches.

yes, we don't make any distinction between open and closed in the thread icon.

reducing our css rules is good (for performance), and removing duplicate images
is good (makes our .jar file smaller, faster to load and reduces the size of the
download.)  good work, hwaara.

next we should also fix classic (same problem there, I checked).  for extra
points, we should rename the files and remove "closed" from the names.  that
doesn't have an impact on performance, so that can be spun off to another bug.

Comment 13

[(not reading, please use [e-mail] instead)]

> next we should also fix classic (same problem there, I checked)

oops, classic is correct as is.  hwaara points out that classic did make the
distinction between open and closed thread icons.  (the thread in the spool was
long or short depending on if the thread was open or closed.)

I still think we should spin up a new bug about removing "closed" from the name
of the modern icon files, but not fix it right now.  it's a minor tweak, more
for our (developer's) benefit.

Comment 14

[Håkan Waara]

bug 147945 posted for the remaining thread-closed image files to be renamed, and
all patches checked in.

Comment 15

[R.K.Aa.]

possible fallout: bug 148040

Comment 16

[R.K.Aa.]

forget comment 15 (is likely a fallout from bug 130544)