Closed Bug 1296955 Opened 3 years ago Closed 3 years ago

Some style rules missing from the mac classic console.css


(SeaMonkey :: General, defect)

Not set


(seamonkey2.47 fixed, seamonkey2.48 fixed)

Tracking Status
seamonkey2.47 --- fixed
seamonkey2.48 --- fixed


(Reporter: stefanh, Assigned: frg)




(1 file, 1 obsolete file)

The mac version of console.css contains css that assumes preprocessing. console.css has ’%include ../’ which is ment to point to toolkit/themes/osx/global/ In, you will find the actual css which substitutes the variables/entities (like @scopeBarBackground@ and so on found in console.css).

There are 2 ways to fix this:
1) Fix the include to point to the right file and make console.css be preprocessed.
2) Substitute the variables/entities with "real css".

I think I prefer 2) since we don't generally preprocess css files in suite/ and since this file is a toolkit file, we don't really know what will happen to it in the future. We could put the variable name from the toolkit file as a comment next to the style rules in console.css so it's easy to find the css if something in changes.
Attached patch 1296955-stylerules.patch (obsolete) — Splinter Review
Sorry missed this one. You better review it too because I am unable to test it.
Assignee: nobody → frgrahl
Attachment #8783584 - Flags: review?(stefanh)
Comment on attachment 8783584 [details] [diff] [review]

Excellent, thanks. Just some nits in order to make the style rules consistent with the rest of the file(s).

>-  background: @roundButtonBackground@;
>-  box-shadow: @roundButtonShadow@;
>+  border: 1px solid rgba(0,0,0,.35); /* @roundButtonBorder@ */
>+  background: linear-gradient(#f6f6f6, #e9e9e9); /* @roundButtonBackground@ */
>+  box-shadow: 0 1px rgba(255,255,255,.5), inset 0 1px 1px rgba(255,255,255,.5); /* @roundButtonShadow@ */
> }

Can you please make the 'f' and 'e' uppercase and add space after the ',':s in the "rgba(...":s

> #ButtonEval:hover:active {
>-  text-shadow: @loweredShadow@;
>-  background: @roundButtonPressedBackground@;
>-  box-shadow: @roundButtonPressedShadow@;
>+  text-shadow: 0 1px rgba(255, 255, 255, .4); /* @loweredShadow@ */
>+  background: #dadada; /* @roundButtonPressedBackground@ */
>+  box-shadow: 0 1px rgba(255,255,255,.4), inset 0 1px 3px rgba(0,0,0,.2); /* @roundButtonPressedShadow@ */
> }

Same thing here ('#DADADA' and space after the commas on last line)

Btw, we have this at the top of the file:

> /* View buttons */
> @import "chrome://global/skin/viewbuttons.css";

It's fine, but I see now that we actually lacks some elements styled in this file, so further tweaks to console.css might be needed, but that's my problem ;-)
Attachment #8783584 - Flags: review?(stefanh) → review+
V2 patch with nits fixed. Review+ from stefanh carried forward.
Attachment #8783584 - Attachment is obsolete: true
Attachment #8783966 - Flags: review+
Comment on attachment 8783966 [details] [diff] [review]

a=me could you please change .4 to 0.4 .5 to 0.5 etc thanks
pushed with 0.

Stefan if the next build is ok for you I will ask for c-a approval.
Whiteboard: [leave open for branches]
(In reply to Frank-Rainer Grahl from comment #5)
> Stefan if the next build is ok for you I will ask for c-a approval.

While reviewing attachment 8783584 [details] [diff] [review], I also did a test spin with it (there is no next build since the mac builds have been broken since June afaik).
Comment on attachment 8783966 [details] [diff] [review]

[Approval Request Comment]
Regression caused by (bug #): 686168
User impact if declined: console will not work on osx due to invalid css
Testing completed (on m-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): none broken already.
String changes made by this patch: none
Attachment #8783966 - Flags: approval-comm-aurora?
Comment on attachment 8783966 [details] [diff] [review]

Attachment #8783966 - Flags: approval-comm-aurora? → approval-comm-aurora+
Hopefully fixed
Closed: 3 years ago
Resolution: --- → FIXED
Whiteboard: [leave open for branches]
Target Milestone: --- → seamonkey2.48
You need to log in before you can comment on or make changes to this bug.