Closed Bug 456757 Opened 16 years ago Closed 15 years ago

[Meta] Update the Modern theme for SeaMonkey 2.0

Categories

(SeaMonkey :: Themes, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: spitfire.kuden)

References

(Depends on 1 open bug, )

Details

(Keywords: fixed-seamonkey2.0, modern)

Attachments

(5 files, 6 obsolete files)

This is a tracking bug for work on updating the Modern Theme and implementing the missing elements now that trunk has moved to tookkit (e.g. /global/ and /mozapps/). There is also a thread in the Theme Development forum on the Mozillazine phpBB tracking this work. http://forums.mozillazine.org/viewtopic.php?f=18&t=748735. Kuden is currently working on /mozapps/
Flags: wanted-seamonkey2?
Flags: wanted-seamonkey2? → wanted-seamonkey2+
Depends on: 432016, 416219
Keywords: modern
Depends on: 458246
Depends on: 279611
Attached file /mozapps/ (deleted) —
Kuden has finished /mozapps/ and posted a full theme (tkmodern) at http://maguroban.s41.xrea.com/theme/ToolkitModern.jar. I think it's too large to attach here so I'm going to attach only the /mozapps/ sub directory. Phil
(In reply to comment #1) > Created an attachment (id=348743) [details] > /mozapps/ At least, these changes are also necessary with /mozapps/ . button.css, textbox.css, wizard.css, toolbarbutton.css, progressmeter.css, richlistbox.css, notification.css and /global/throbber/ /global/icons/(added new icons)
Note that we don't actually use downloads or handling (though this may change), passwordmgr, places or profile.
The .jar also seems to include updated theming for newsblog and some other pages (ThreeDShadow crept into config.css and netError.css for example...)
Assignee: nobody → spitfire.kuden
> (ThreeDShadow crept into config.css and netError.css for example...) Is that good or is that bad?
It's bad that we currently have ThreeDShadow in Modern, yes.
ThreeDShadow is mapped to a system color that varies between platforms. So, it won't be the same on different platforms.
(In reply to comment #4) > The .jar also seems to include updated theming for newsblog Oops, that was a bogus diff on my part. At a quick glance, the change to global/button.css is incorrect, though.
The about:buildconfig changes look nice, except for the outer netError-like border, which I don't think fits, and doesn't really work with about: either.
Attached patch global/config.css (deleted) — Splinter Review
I liked some of your config.css changes: * use background-color not just background * #warningBox needs a colour * ThreeDShadow should be a Modern colour * #exclam should have a width and height but not others: * comments restored * bracing and CSS properties reverted to file style * useless and incorrect #configTree margin removed * #filterRow margin removed, I don't think we need it What do you think?
Attachment #349042 - Flags: review?(spitfire.kuden)
(In reply to comment #10) > Created an attachment (id=349042) [details] > global/config.css This looks good. However, I cannot change the attachment (id=349042)... Neil, Does mozapps of SeaMonkey Modern need only these? mozapps/extensions/ mozapps/plugins/ mozapps/update/ mozapps/xpinstall/ (I memorize for mozapps/places/ to have been made by demanding KaiRo.) I select only files really necessary for mozapps of SeaMonkey Modern.
I should be filing several dependent bugs for each item as this should be kept as the META or tracking bug. (In reply to comment #2) > At least, these changes are also necessary with /mozapps/ . > button.css, textbox.css, wizard.css, toolbarbutton.css, > progressmeter.css, richlistbox.css, notification.css > and > /global/throbber/ > /global/icons/(added new icons) (In reply to comment #11) > However, I cannot change the attachment (id=349042)
Blocks: 465924
I've filed Bug 465924 for work on global. Neil could you copy/move your attachment(s) there? (In reply to comment #11) > However, I cannot change the attachment (id=349042) Neil can we arrange for Kuden to get editbugs?
Attachment #349042 - Flags: review+
Attachment #349042 - Flags: review?(spitfire.kuden)
(In reply to comment #11) > Neil, Does mozapps of SeaMonkey Modern need only these? > mozapps/extensions/ > mozapps/plugins/ > mozapps/update/ > mozapps/xpinstall/ It definitely needs these. It might need some of the others later too. (In reply to comment #13) > I've filed Bug 465924 for work on global. Neil could you copy/move your > attachment(s) there? Hardly seems worth it for attachment 348042 [details].
Comment on attachment 349042 [details] [diff] [review] global/config.css Pushed changeset bbb774695612 to comm-central.
Depends on: 355822
Removing dependency on Bug 355822 for the moment since we are using a forked UI for the profile selection dialog (we use a tree instead of a listbox). Can someone check if this affects us?
No longer depends on: 355822
Depends on: 367272
(In reply to comment #16) > Removing dependency on Bug 355822 for the moment since we are using a forked UI > for the profile selection dialog (we use a tree instead of a listbox). Can > someone check if this affects us? It should have no affect on us at all.
Do we already have a bug for icons in the Error Console (All, Errors, Warnings, Messages, Clear) or is this not desired for Modern?
> Do we already have a bug for icons in the Error Console (All, Errors, Warnings, > Messages, Clear) or is this not desired for Modern? Well traditionally we didn't have icons in the XPFE error console. When we moved to toolkit we picked up the toolkit error console with the fancy toolbar buttons - but only with classic of course (since much of classic comes from toolkit). I'm the current maintainer of Console² which has icons and customizable toolbars so a current workaround for you is to install this extension.
(In reply to comment #18) > Do we already have a bug for icons in the Error Console (All, Errors, Warnings, > Messages, Clear) or is this not desired for Modern? Well, I don't mind, as long as we can turn them off again ;-)
Depends on: 471921
No longer depends on: 458246
Depends on: 458246
Blocks: 477309
Depends on: 477427
No longer blocks: 477309
Depends on: 477710, 477309
From what I heard so far, we'd need some work from someone who can drive the existing work done by Kuden through our review process and into the SeaMonkey tree, which will probably require a number of (smaller) adaptations.
Keywords: helpwanted
Attached patch MozApps from Kuden's Modern Theme (obsolete) (deleted) — Splinter Review
Attachment #367725 - Flags: review?
Attached patch Reporter CSS Changes (from Kuden) (obsolete) (deleted) — Splinter Review
Attachment #367726 - Flags: review?
Attached patch Messenger CSS and Throbber PNG (from Kuden) (obsolete) (deleted) — Splinter Review
Attachment #367727 - Attachment description: Messenger CSS and Throbber PNG → Messenger CSS and Throbber PNG (from Kuden)
Attachment #367727 - Flags: review?
Attachment #367726 - Attachment description: Reporter CSS Changes → Reporter CSS Changes (from Kuden)
Attached file Global CSS Changes (Bad Upload) (obsolete) (deleted) —
Attachment #367728 - Flags: review?
Attachment #367728 - Attachment description: Global CSS Changes → Global CSS Changes (Bad Upload)
Attachment #367728 - Attachment is obsolete: true
Attachment #367728 - Flags: review?
Attached patch Global CSS Changes (from Kuden) (obsolete) (deleted) — Splinter Review
Attached patch Modern Directory's CSS and PNGs (from Kuden) (obsolete) (deleted) — Splinter Review
Attachment #367730 - Flags: review?
I just finished uploading all of the patches from extracting the JAR onto a Comm-Central checkout. I'll investigate who best to review and if any files were deleted tomorrow. I'll also upload zip files of the images tomorrow as well. Let me know if there is a preferred way to break up the changes/additions.
One problem is that we want to keep the changes (bug 474807 is one obvious example) to Modern that have been made since the .jar was created.
In fact bug 474807 is a really annoying example as it also need to be applied to the "new" parts to the theme :-(
Comment on attachment 367725 [details] [diff] [review] MozApps from Kuden's Modern Theme Benjamin, please direct the review to a specific person, review requests without a target person are usually ignored as nobody feels it's his task to look at them. Setting them to Neil as he's our "UI tsar" and also knows Modern quite well.
Attachment #367725 - Flags: review? → review?(neil)
Attachment #367726 - Flags: review? → review?(neil)
Attachment #367727 - Flags: review? → review?(neil)
Attachment #367730 - Flags: review? → review?(neil)
Benjamin: Do you plan to work on what Neil said in Comment 29? If you need some help with this, please comment in the bug.
I do plan to work on what Neil said in comment 29. I am aiming to have anther round of patches ready this week. Sorry for the delay.
Due to exams and the death of what was a faulty NVIDIA graphics chip, I am not able to get to this until this weekend. If that is a problem, feel free to take the bug from me. My planned approach is to start with the patches I created and then add all of the patches made to modern since the theme was created, according to the results of a search for "modern" on http://hg.mozilla.com/comm-central. Does this seem the best way to do this? Also, I assume that it is actually preferred that I submit patches on the dependent bugs. If so, I will do that for the next round of patches.
Attachment #367725 - Attachment is obsolete: true
Attachment #367725 - Flags: review?(neil)
Attachment #367730 - Attachment is obsolete: true
Attachment #367730 - Flags: review?(neil)
Attachment #367729 - Attachment is obsolete: true
Attachment #367727 - Attachment is obsolete: true
Attachment #367727 - Flags: review?(neil)
Attachment #367726 - Attachment is obsolete: true
Attachment #367726 - Flags: review?(neil)
> My planned approach is to start with the patches I > created and then add all of the patches made to modern since the theme was > created, according to the results of a search for "modern" on > http://hg.mozilla.com/comm-central. Does this seem the best way to do this? Errr. Nooo. I would recommend instead that you do a manual merge (I think some people use kdiff3, I'm on Windows so I use WinMerge) file by file making sure that you don't override any changes to the theme that were made since Kuden made the modified JAR. You can use MXR to look at each file, then for each file in the top right corner are links to the HG log. The column on the left in the HG log has a "diff" link for each changeset that tells you what was changed. > Also, I assume that it is actually preferred that I submit patches on the > dependent bugs. Yes that's the way to do it. Also if there doesn't exist a dependent bug for a particular global submodule then by all means file one and make it block this one. Keep up the good work!
No longer blocks: 465924
Depends on: 465924
Depends on: 398138
Depends on: 490277
Depends on: 493022
Depends on: 498153
Ping?
Benjamin: Any update?
Attachment #367728 - Attachment mime type: application/octet-stream → text/plain
No longer depends on: 416219
Depends on: 512254
No longer depends on: 432016
Attached patch Fix Modern iconic menus (deleted) — Splinter Review
Steps to reproduce problem: 1. Install an extension that uses iconic menus e.g. Versions (https://addons.mozilla.org/addon/3830) 2. Switch to the Modern theme 3. Note that the arrow for the iconic menu does not line up. This is because the XBL for iconic menus includes pack="center" but not for ordinary menus. The problem does not occur in the Classic theme on either Windows or Linux because both of those themes use -moz-appearance on the containing box. The problem is less obvious with Pinstripe because of its native menus. OS/2 also fixed the problem by adding some padding to its containing box.
Attachment #405047 - Flags: review?(iann_bugzilla)
Dao suggested increasing the margin and reducing the width instead.
Attachment #405050 - Flags: review?(iann_bugzilla)
Comment on attachment 405050 [details] [diff] [review] Alternative fix as suggested by Dao I think I prefer this one as the actual image is 4px wide
Attachment #405050 - Flags: review?(iann_bugzilla) → review+
Attachment #405047 - Flags: review?(iann_bugzilla)
Attachment #405050 - Flags: approval-seamonkey2.0+
Let's get this off the "approved but not fixed" radar for 2.0 - I wonder if we should mark it fixed as a whole...
oops, apparently there is an actual patch here that was not checked in, so it correctly shows on the radar. I hate intermingling trackers with actual patches on the bug...
Depends on: 521383
Pushed changeset 9bad4209e8aa to mozilla-central. I know this is really a tracking bug, but I'm going to mark it fixed anyway.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Keywords: helpwanted
Attached patch Another 1-liner (deleted) — Splinter Review
Sorry for being too lazy to file separate bugs for all these. This changes the display of password requests to a nice "lock" icon; by comparison Linux builds already use a "key" instead of a question mark.
Attachment #406018 - Flags: review?(iann_bugzilla)
Attachment #406018 - Flags: review?(iann_bugzilla)
Attachment #406018 - Flags: review+
Attachment #406018 - Flags: approval-seamonkey2.0+
Comment on attachment 406018 [details] [diff] [review] Another 1-liner Pushed changeset 98c8cbfc4b23 to comm-central.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: