Closed Bug 225090 Opened 21 years ago Closed 21 years ago

Sanitize Help CSS

Categories

(SeaMonkey :: Help Documentation, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.6final

People

(Reporter: danielwang, Assigned: rjkeller)

Details

Attachments

(1 file, 2 obsolete files)

We need:

/* for <td><p> and <li><p>, in case we have multiple <p>s */
p:first-child { padding-top: 0; margin-top: 0; }

/* for long lists, bigger separation make items more pleasing
to read and more clear */
li { padding-bottom: 0.5em; }

ul.compact li, ol.compact li { padding-bottom: 0; }

/* UI element labels, e.g. menu items */
.ui { color: #3333cc; }

h1, h2, h3, h4, h5, h6 {
//  color: blah
}

strong { color: blah; }
em { font-style: normal; font-weight: bold; }

kbd { font-family: sans-serif; }

--------

We need to consider not using <kbd>. The reason is that x-height of normal
sans-serif and monospace are different. On Windows (and possibly Linux),
x-height of monospace is especially small, thus creating reading difficulty. The
breakage is especially severe when we have a whole sentence in sans-serif but
some words in monospace (e.g. "to bookmark the current page, type _ctrl+b_. blah
blah). For longer phrases, the eyes can more easily adjust to the type change
(e.g. "To search the Web, type _http://www.google.com_ in the Location Bar).
sounds good. Let's do this during 1.7alpha.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
> /* for long lists, bigger separation make items more pleasing
> to read and more clear */
> li { padding-bottom: 0.5em; }
>
> ul.compact li, ol.compact li { padding-bottom: 0; }

Let's switch this. Let's have a .expand or something like that that will have
padding-bottom: 0.5em;, while the default is 0.

> h1, h2, h3, h4, h5, h6 {
> //  color: blah
> }

why set this?

> strong { color: blah; }
> em { font-style: normal; font-weight: bold; }

why are we changing <em> from italic to bold?
Target Milestone: Future → ---
Daniel, can you please answer my comments :). I'd like to get this in for 1.6b 
if we can.
>> h1, h2, h3, h4, h5, h6 {
>> //  color: blah
>> }
>
> why set this?

shrug. sounded good when I filed the bug.

>> strong { color: blah; }
>> em { font-style: normal; font-weight: bold; }
>
> why are we changing <em> from italic to bold?

because italic is ugly

for .ui, background-color: is probably better than color:
> because italic is ugly

Then maybe we need to add something different than italic.

> for .ui, background-color: is probably better than color:

Think we should highlight UI items that much? Anyway, that color you specified
in the report is too dark to make a decent background color, nobody could see
the text. Maybe #B7F5C7 would be a less painful background color.
Attached patch Patch (obsolete) — Splinter Review
Attachment #136201 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 136201 [details] [diff] [review]
Patch

what the heck? Not sure what I attached :). Let's try again.
Attachment #136201 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch Real Patch ;) (obsolete) — Splinter Review
real patch ;)
Attachment #136201 - Attachment is obsolete: true
Attachment #136317 - Flags: review?(neil.parkwaycc.co.uk)
> +.separate li { padding-bottom: 0.5em; }

What is this for? Please comment the source ;-)
> What is this for? Please comment the source ;-)

That spacing was the opposite that you reported in the bug. Instead of having a
class that would make the lists not have spacing, I made one so that they did so
that the old docs wouldn't get messed up with the new spacing.
Component: User → Help
Product: Documentation → Browser
Target Milestone: --- → mozilla1.6beta
Version: unspecified → Trunk
Comment on attachment 136317 [details] [diff] [review]
Real Patch ;)

r=timeless over IRC, BTW.
Attachment #136317 - Flags: review?(neil.parkwaycc.co.uk) → approval1.6b?
Comment on attachment 136317 [details] [diff] [review]
Real Patch ;)

>+.separate li { padding-bottom: 0.5em; }

Why not margin-bottom?

>+.ui { background-color: #B7F5C7 }

You could use CSS2 system colors...
Comment on attachment 136317 [details] [diff] [review]
Real Patch ;)

a=asa (on behalf of drivers) for checkin to 1.6beta
Attachment #136317 - Flags: approval1.6b? → approval1.6b+
> You could use CSS2 system colors...

dbaron, do you mean using rgb(x,y,z);?

Changing Target Milestone to 1.6, I'd like to debug this more on other help
files before checking this in.
Target Milestone: mozilla1.6beta → mozilla1.6final
which is deprecated in favour of http://www.w3.org/TR/css3-ui/#system

Do UI labels have to look like real UI elements? If we want this, maybe we
should also style the text and background to window text and window background
colours. but that may look ugly.

>>+.separate li { padding-bottom: 0.5em; }
> 
> Why not margin-bottom?

yeah, padding will be a problem if we ever want to use border or background. We
should change this.
The fact that it's deprecated is irrelevant.  (I objected to the deprecation,
actually, and I should do so more forcefully.)  Mozilla implements them, and
they're useful.
> >+.ui { background-color: #B7F5C7 }
> 
> You could use CSS2 system colors...

I disagree. I would like consistency in help and not have different colors on
different OS's or different OS settings.

I definetely agree on the padding being changed to margin.
Attachment #136317 - Attachment is obsolete: true
Comment on attachment 137773 [details] [diff] [review]
Patch with dbaron's comments

dbaron, since you offered feedback on this, can you review this?
Attachment #137773 - Flags: review?(dbaron)
Comment on attachment 137773 [details] [diff] [review]
Patch with dbaron's comments

>+.separate li { margin-bottom: 0.5em; }

r=dbaron, although this should probably be:

.separate > li { ...
Attachment #137773 - Flags: review?(dbaron) → review+
Fix checked in.

dbaron, I changed ".separate li" to ".separate > li" before checkin.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: