Closed Bug 916478 Opened 7 years ago Closed 7 years ago

Make our css more efficient and remove some obsolete style rules

Categories

(SeaMonkey :: Themes, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.23

People

(Reporter: stefanh, Assigned: stefanh)

References

Details

Attachments

(1 file, 3 obsolete files)

I noticed bug 915909, comment #9 - so I figured a file bug for this. I've also noticed a bunch of obsolete rules.
Attached patch v1.0 (obsolete) — Splinter Review
Not yet tested. I found quite a few obsolete rules...
Attached patch v1.1 (obsolete) — Splinter Review
I removed some obsolete rules. When it comes to the window.dialog rule and the description.error rule in communicator/profile/profile.css i figured the padding wouldn't have been nice if it had worked and I didn't understood the description.error rule (I can put it back, but I don't see why it should be a different colour).

dialog.css is now empty in classic since I couldn't find any .push menus (I guess -moz-appearance would overrule some of the rules if we had .push menus)
Attachment #804922 - Attachment is obsolete: true
Attachment #804929 - Flags: superreview?(neil)
Attachment #804929 - Flags: review?(neil)
Status: NEW → ASSIGNED
(In reply to Stefan from comment #2)
> I removed some obsolete rules. When it comes to the window.dialog rule and
> the description.error rule in communicator/profile/profile.css i figured the
> padding wouldn't have been nice if it had worked and I didn't understood the
> description.error rule (I can put it back, but I don't see why it should be
> a different colour).
window.dialog was obsolete since a Ben Goodger change on 2000-03-28, but description.error is still a real class and should be kept (actually it should be added somewhere else too so that pref-directory-add.xul can use it.)

> dialog.css is now empty in classic since I couldn't find any .push menus (I
> guess -moz-appearance would overrule some of the rules if we had .push menus)
I can only guess that .push menus were obsoleted by button type="menu".
Attached patch v1.2 (obsolete) — Splinter Review
I added the description.error rule in communicator.css (messenger.css imports communicator.css and profileSelection.xul uses communicator.css)

The other option is to put the rule back in profile.css and add it in messenger.css (used by pref-directory-add.xul).
Attachment #804929 - Attachment is obsolete: true
Attachment #804929 - Flags: superreview?(neil)
Attachment #804929 - Flags: review?(neil)
Attachment #805002 - Flags: superreview?(neil)
Attachment #805002 - Flags: review?(neil)
Hmm, profileSelection.xul have this:
<description id="intro" style="width: 17em;"

and profile.css has this:
#intro {
  width: 17em;
}

There are some other hardcoded style rules in that dialog that looks strange. I can open a separate bug for that.
Comment on attachment 805002 [details] [diff] [review]
v1.2

Oh, I see that I missed treecell.importsampledata which does looks like it could be used without the element name.
Attachment #805002 - Flags: superreview?(neil)
Attachment #805002 - Flags: review?(neil)
Attached patch Final versionSplinter Review
See comment #4 and comment #6.
Attachment #805002 - Attachment is obsolete: true
Attachment #805006 - Flags: superreview?(neil)
Attachment #805006 - Flags: review?(neil)
OS: Mac OS X → All
Comment on attachment 805006 [details] [diff] [review]
Final version

Looks like the syncIcon element got removed in bug 684537 (port of bug 626949) but the CSS was forgotten about.
Attachment #805006 - Flags: superreview?(neil)
Attachment #805006 - Flags: superreview+
Attachment #805006 - Flags: review?(neil)
Attachment #805006 - Flags: review+
http://hg.mozilla.org/comm-central/rev/58c86e8a42fb

Then I noticed an error in one of the "error messages" comments which I fixed by pushing
http://hg.mozilla.org/comm-central/rev/40c0e9b4ab21 (yes, I messed up the commit msg a bit)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.23
Comment on attachment 805006 [details] [diff] [review]
Final version

>-<?xml-stylesheet href="chrome://messenger/skin/dialogs.css" type="text/css"?>
>-
Oops, it was wrong to remove these completely, instead just the dialogs.css should have been removed, leaving chrome://messenger/skin/. This broke message print preview for example.
Depends on: 918925
You need to log in before you can comment on or make changes to this bug.