Closed Bug 1699322 Opened 2 months ago Closed 1 month ago

Convert gif icons in SeaMonkey themes to png.

Categories

(SeaMonkey :: Themes, enhancement)

enhancement

Tracking

(seamonkey2.53? affected, seamonkey2.57esr? affected)

RESOLVED FIXED
seamonkey 2.86
Tracking Status
seamonkey2.53 ? affected
seamonkey2.57esr ? affected

People

(Reporter: frg, Assigned: frg)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: SM2.53.8)

Attachments

(10 files, 6 obsolete files)

395.38 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-release+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
42.83 KB, patch
Details | Diff | Splinter Review
564.70 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-release+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
37.30 KB, patch
frg
: review+
Details | Diff | Splinter Review
464.92 KB, patch
frg
: review+
Details | Diff | Splinter Review
509.97 KB, patch
frg
: review+
Details | Diff | Splinter Review
81.63 KB, patch
frg
: review+
Details | Diff | Splinter Review
3.46 KB, patch
frg
: review+
Details | Diff | Splinter Review
66.80 KB, patch
frg
: review+
Details | Diff | Splinter Review
467.91 KB, patch
frg
: review+
Details | Diff | Splinter Review

+++ This bug was initially created as a clone of Bug #1698675 +++

According to the list buc provided in Bug 1698675 some of our gif icons might cause performance or other problems. Firefox and Thunderbird switched to png and svg in latest release. We still have a mix of gif and png icons and should consolidate this for uniformity.

buc tracked down a possible performance problem with to 16x.10 gif icon. Gif icons will be replaced overtime with later formats so let take the opportunity to do this here first for the statusbar and menu bar icon.
The status bar icon also is 16x10 and can just be the same as the 16x16 menu bar icon.

Trivial fix so can go into 2.53.7 final release.

+++ snip +++

A broken choice of chatzilla's taskbar icon leads to a constant redrawing of the screen.

Disable ALL the extensions (chatzilla, inspector, lightning) in about:addons, and run SeaMonkey from cmdline with debug:

MOZ_LOG="nsRefreshDriver:5" seamonkey

The debug output stopped after the page is completed, until you move mouse etc., then it stopped again etc.etc It is an expected behaviour.

Then enable chatzilla in about:addons and repeat the MOZ_LOG run.

After the page is completed, the debug output still present infinitely, despite you do nothing.

The problem is suite/extensions/irc/xul/skin/images/taskbar-irc.gif icon, with size 16x10 .
It seems the code require geometry of 16x16 in this area. If you try to use png or jpg icon with another size there, it just not appear. But in the case of gif, the icon with the size of 16x10 (instead of 16x16) still appear, with a side effect of permanent refreshing of the screen (according to the debug output).

Attached patch 1699322-1-gif2png-2538.patch (obsolete) — Splinter Review

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: The world will end
Testing completed (on m-c, etc.): 2.53.8b1 pre modern and classic
Risk to taking this patch (and alternatives if risky): trivial conversion
String changes made by this patch: --

This adds about 500K to the installer. Most of the smaller icons almost double in size. Some of the bigger ones shrink but not enough of them in the tree to offset the gain.

Patch applied at the end of the current patch chain. Bug 1296850 need to be moved after it.

Attachment #9209916 - Flags: review?(iann_bugzilla)
Attachment #9209916 - Flags: approval-comm-release?
Attachment #9209916 - Flags: approval-comm-esr60?
Blocks: 1296850
Attached patch 1699322-2-gif2png-2538.patch (obsolete) — Splinter Review

css and packaging changes. duplicate list goes into another patch.

find . -name ".css" | xargs sed -i -e '/communicator/s/.gif/.png/'
find . -name "
.css" | xargs sed -i -e '/messenger/s/.gif/.png/'
find . -name ".css" | xargs sed -i -e '/editor/s/.gif/.png/'
find . -name "
.css" | xargs sed -i -e '/navigator/s/.gif/.png/'

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: The world has already ended with patch 1 so no impact.
Testing completed (on m-c, etc.): 3.53.8b1 pre
Risk to taking this patch (and alternatives if risky): trivial replacement.
String changes made by this patch: --

Attachment #9209917 - Flags: review?(iann_bugzilla)
Attachment #9209917 - Flags: approval-comm-release?
Attachment #9209917 - Flags: approval-comm-esr60?
Attached patch 1699322-3-gif2png-2538.patch (obsolete) — Splinter Review

[Approval Request Comment]
Regression caused by (bug #): various
User impact if declined: none warning only
Testing completed (on m-c, etc.): 2.53.8b1 pre
Risk to taking this patch (and alternatives if risky): trivial. We use the -w flag which makes this non-fatal
String changes made by this patch: --

Attachment #9209924 - Flags: review?(iann_bugzilla)
Attachment #9209924 - Flags: approval-comm-release?
Attachment #9209924 - Flags: approval-comm-esr60?
Attached patch 1699322-domi-gif2png-2538.patch (obsolete) — Splinter Review

Do the same for DOMi. No longer has a separate component so just add to this bug. 2.53.branch only.

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: unfinished business
Testing completed (on m-c, etc.): 2.53.8b1 pre
Risk to taking this patch (and alternatives if risky): trivial
String changes made by this patch: --

Attachment #9209933 - Flags: review?(iann_bugzilla)
Attachment #9209933 - Flags: approval-comm-release?
Attached patch 1699322-3-gif2png-2538.patch (obsolete) — Splinter Review

[Approval Request Comment]
Regression caused by (bug #): various
User impact if declined: none warning only
Testing completed (on m-c, etc.): 2.53.8b1 pre
Risk to taking this patch (and alternatives if risky): trivial. We use the -w flag which makes this non-fatal
String changes made by this patch: --

Attachment #9209924 - Attachment is obsolete: true
Attachment #9209924 - Flags: review?(iann_bugzilla)
Attachment #9209924 - Flags: approval-comm-release?
Attachment #9209924 - Flags: approval-comm-esr60?
Attachment #9209934 - Flags: review?(iann_bugzilla)
Attachment #9209934 - Flags: approval-comm-release?
Attachment #9209934 - Flags: approval-comm-esr60?

(In reply to Frank-Rainer Grahl (:frg) from comment #0)

buc tracked down a possible performance problem with to 16x0 gif icon.

Nope.
A performance problem with any size gif icon with non-zero delay set (as seems normally produced by gimp).

This adds about 500K to the installer. Most of the smaller icons almost double in size.

It is strange a bit...

My tests on small gif icons (fe. chatzilla-16.gif) shows the size increased about 35% (1.35 < 2.0). I used gimp for conversions.

Maybe there are some stuff -- internal comments etc. -- which can be omitted in the result images?

The cost of additional 500k with no user visible effect should be avoided if possible...

If ImageMagick was used for gif2png conversion, then by default it increases the size up to twice. But if you use "-strip" option (removes mostly some text comments in the file), then the resulting png size is increased about 10% only.

IOW, "convert filename.gif -strip filename.png"

Flags: needinfo?(frgrahl)

I used RWPaint and stripped the extras. Didn't notice first that it were that many files but when I realized it was faster to convert the remaining ones manually. Most files are really tiny and I didn't see bloat in them afterwards. Size increase is only about 200K with a fresh build. It is 2021 so not care much about this imho tiny overall increase.

Flags: needinfo?(frgrahl)
Duplicate of this bug: 1699634

Add back global gifs to the modern theme for compatibility with toolkit css. Let it bake a day or two before asking for review again.

Attachment #9209916 - Attachment is obsolete: true
Attachment #9209916 - Flags: review?(iann_bugzilla)
Attachment #9209916 - Flags: approval-comm-release?
Attachment #9209916 - Flags: approval-comm-esr60?

Switch "global" gifs in modern to png.

In modern only: find . -name "*.css" | xargs sed -i -e '/global/s/.gif/.png/'

Attachment #9209917 - Attachment is obsolete: true
Attachment #9209917 - Flags: review?(iann_bugzilla)
Attachment #9209917 - Flags: approval-comm-release?
Attachment #9209917 - Flags: approval-comm-esr60?

Add section for compatibility gif files

Attachment #9209934 - Attachment is obsolete: true
Attachment #9209934 - Flags: review?(iann_bugzilla)
Attachment #9209934 - Flags: approval-comm-release?
Attachment #9209934 - Flags: approval-comm-esr60?

Comment on attachment 9209933 [details] [diff] [review]
1699322-domi-gif2png-2538.patch

Isn't 2.0.17.3 sufficient as a bump?

r/a=me either way

Attachment #9209933 - Flags: review?(iann_bugzilla)
Attachment #9209933 - Flags: review+
Attachment #9209933 - Flags: approval-comm-release?
Attachment #9209933 - Flags: approval-comm-release+

Last time I converted images from GIF to PNG I ran them through optipng which does a good basic job of optimising images, but there's also advpng and deflopt if you want to get really serious.

Compared the pngs buc provided and the ones I did. Was not 100% one sided so took only the smaller ones. Fixed up the gifs kept for compatibility which had a delay in according to buc also.

For the gifs PMView Pro produced the smallest ones.

Didn't try the tools neil used yet but most pngs are so small it would imho not matter. Might be worth to check it out for the larger ones later.

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: The world will end again
Testing completed (on m-c, etc.): 2.53.8b1 pre modern and classic
Risk to taking this patch (and alternatives if risky): trivial conversion
String changes made by this patch: --

Attachment #9210291 - Attachment is obsolete: true
Attachment #9210743 - Flags: review?(iann_bugzilla)
Attachment #9210743 - Flags: approval-comm-release?
Attachment #9210743 - Flags: approval-comm-esr60?

Comment on attachment 9210292 [details] [diff] [review]
1699322-2-gif2png-v1_1-2538.patch

Unchanged
[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: The world has already ended with patch 1 so no impact.
Testing completed (on m-c, etc.): 3.53.8b1 pre
Risk to taking this patch (and alternatives if risky): trivial replacement.
String changes made by this patch: --

Attachment #9210292 - Flags: review?(iann_bugzilla)
Attachment #9210292 - Flags: approval-comm-release?
Attachment #9210292 - Flags: approval-comm-esr60?
Depends on: 1700003

Patch update for change of DOMI location to suite/extensions/inspector.

Removed the install.rdf version changes. Unrelated to this bug and we can clean it up in a new DOMi bug after the 2.53 integration now in effect.

Kept 2.18 version. Unlikely that this will rollover ever.

r/a+ from IanN retained.

Attachment #9209933 - Attachment is obsolete: true
Attachment #9210745 - Flags: review+
Attachment #9210745 - Flags: approval-comm-release+

Comment on attachment 9210292 [details] [diff] [review]
1699322-2-gif2png-v1_1-2538.patch

[Triage Comment]
LGTM r/a=me

Attachment #9210292 - Flags: review?(iann_bugzilla)
Attachment #9210292 - Flags: review+
Attachment #9210292 - Flags: approval-comm-release?
Attachment #9210292 - Flags: approval-comm-release+
Attachment #9210292 - Flags: approval-comm-esr60?
Attachment #9210292 - Flags: approval-comm-esr60+

Comment on attachment 9210743 [details] [diff] [review]
1699322-1-gif2png-v1_2-2538.patch

[Triage Comment]
follow-up can be with optipng r/a=me

Attachment #9210743 - Flags: review?(iann_bugzilla)
Attachment #9210743 - Flags: review+
Attachment #9210743 - Flags: approval-comm-release?
Attachment #9210743 - Flags: approval-comm-release+
Attachment #9210743 - Flags: approval-comm-esr60?
Attachment #9210743 - Flags: approval-comm-esr60+

Modern png files run through optipng

Attachment #9210895 - Flags: review?(frgrahl)
Attachment #9210895 - Flags: approval-comm-release?
Attachment #9210895 - Flags: approval-comm-esr60?

Classic png files run through optipng

Attachment #9210896 - Flags: review?(frgrahl)
Attachment #9210896 - Flags: approval-comm-release?
Attachment #9210896 - Flags: approval-comm-esr60?

png files under suite/extensions run through optipng

Attachment #9210897 - Flags: review?(frgrahl)
Attachment #9210897 - Flags: approval-comm-release?
Attachment #9210897 - Flags: approval-comm-esr60?

editor png files run through optipng

Attachment #9210899 - Flags: review?(frgrahl)
Attachment #9210899 - Flags: approval-comm-release?
Attachment #9210899 - Flags: approval-comm-esr60?

help png files run through optipng

Attachment #9210901 - Flags: review?(frgrahl)
Attachment #9210901 - Flags: approval-comm-release?
Attachment #9210901 - Flags: approval-comm-esr60?

branding png files run through optipng

Attachment #9211996 - Flags: review?(frgrahl)
Attachment #9211996 - Flags: approval-comm-release?
Attachment #9211996 - Flags: approval-comm-esr60?

Comment on attachment 9210895 [details] [diff] [review]
1699322-5-modern-optipng-2538.patch

LGTM

Attachment #9210895 - Flags: review?(frgrahl)
Attachment #9210895 - Flags: review+
Attachment #9210895 - Flags: approval-comm-release?
Attachment #9210895 - Flags: approval-comm-release+
Attachment #9210895 - Flags: approval-comm-esr60?
Attachment #9210895 - Flags: approval-comm-esr60+

Comment on attachment 9210896 [details] [diff] [review]
1699322-6-classic-optipng-2538.patch

LGTM

Attachment #9210896 - Flags: review?(frgrahl)
Attachment #9210896 - Flags: review+
Attachment #9210896 - Flags: approval-comm-release?
Attachment #9210896 - Flags: approval-comm-release+
Attachment #9210896 - Flags: approval-comm-esr60?
Attachment #9210896 - Flags: approval-comm-esr60+

Comment on attachment 9210897 [details] [diff] [review]
1699322-7-extensions-optipng-2538.patch

LGTM
The inspector parts need to be stripped for ESR60 and comm-central.

Attachment #9210897 - Flags: review?(frgrahl)
Attachment #9210897 - Flags: review+
Attachment #9210897 - Flags: approval-comm-release?
Attachment #9210897 - Flags: approval-comm-release+
Attachment #9210897 - Flags: approval-comm-esr60?
Attachment #9210897 - Flags: approval-comm-esr60+

Comment on attachment 9210899 [details] [diff] [review]
1699322-8-editor-optipng-2538.patch

LGTM

Attachment #9210899 - Flags: review?(frgrahl)
Attachment #9210899 - Flags: review+
Attachment #9210899 - Flags: approval-comm-release?
Attachment #9210899 - Flags: approval-comm-release+
Attachment #9210899 - Flags: approval-comm-esr60?
Attachment #9210899 - Flags: approval-comm-esr60+

Comment on attachment 9210901 [details] [diff] [review]
1699322-9-help-optipng-2538.patch

LGTM

Attachment #9210901 - Flags: review?(frgrahl)
Attachment #9210901 - Flags: review+
Attachment #9210901 - Flags: approval-comm-release?
Attachment #9210901 - Flags: approval-comm-release+
Attachment #9210901 - Flags: approval-comm-esr60?
Attachment #9210901 - Flags: approval-comm-esr60+

Comment on attachment 9211996 [details] [diff] [review]
1699322-10-branding-optipng-2538.patch

LGTM

Picks up some images from Bug 1362210 which I moved into the patch there and will be stripped for checkin.

Attachment #9211996 - Flags: review?(frgrahl)
Attachment #9211996 - Flags: review+
Attachment #9211996 - Flags: approval-comm-release?
Attachment #9211996 - Flags: approval-comm-release+
Attachment #9211996 - Flags: approval-comm-esr60?
Attachment #9211996 - Flags: approval-comm-esr60+

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/ea59c0e23cfe
Part 1: convert suite gif icons to png. r=IanN
https://hg.mozilla.org/comm-central/rev/8a29fe88a336
Part 2: change gif icons to png in package and css files. r=IanN
https://hg.mozilla.org/comm-central/rev/a1779e29b94e
Part3: Adjust allowed-dupes for latest changes. r=IanN
https://hg.mozilla.org/comm-central/rev/08c5a007d50e
Convert gif icons in SeaMonkey themes to png - modern png run through optipng. r=frg
https://hg.mozilla.org/comm-central/rev/914d6556a09b
Convert gif icons in SeaMonkey themes to png - classic png run through optipng. r=frg
https://hg.mozilla.org/comm-central/rev/27387277f0ac
Convert gif icons in SeaMonkey themes to png - extensions png run through optipng. r=frg
https://hg.mozilla.org/comm-central/rev/94f6b23dd842
Convert gif icons in SeaMonkey themes to png - editor png run through optipng. r=frg
https://hg.mozilla.org/comm-central/rev/f079347a04ce
Convert gif icons in SeaMonkey themes to png - help png run through optipng. r=frg
https://hg.mozilla.org/comm-central/rev/82ff688e3b1a
Convert gif icons in SeaMonkey themes to png - branding run through optipng. r=frg

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey 2.86
You need to log in before you can comment on or make changes to this bug.