336997 - Correct Toolkit theme mistakes on OS/2

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Peter Weilbacher]

Ever since the checkin to winstripe's menu.css on 2005-10-06 17:31 from Bug 243078 arrows on submenus and checkmarks on menuitems of Toolkit apps (at least of Firefox) were lost on OS/2 and I forgot to rectify that in Bug 314843.

As there are other problems with the appearance on OS/2 (e.g. Bug 247161 that goes nowhere), let me try to address these issues here.

Comment 1

[Peter Weilbacher]

Attachment: Screenshot with changes

On this screenshot, note the following things:
- The check marks in the menus are back and now look like they
  do on OS/2 (and are the same as the radio marks in the menus)
- The same for the submenu arrows
- The scrollbar arrows are centered and look OS/2-like
- The scrollbar is as narrow as it is on OS/2 natively
- The scrollbar background has the correct OS/2 color
- The scrollbar thumb (is that the word for the sliding box in
  the middle of the scrollbar?) looks pressed down when clicked
  (not visible on screenshot)
- The scrollbar has the hightlights/shadows on its borders (to
  better separate it from content, this is also how it looks like
  in native OS/2 apps)

Comment 2

[Peter Weilbacher]

Attachment: CSS patch

These are the necessary CSS changes. The file xulscrollbars.css is new and adapted from the copied winstripe version. I will upload the necessary icons in a minute.

Disadvantages and questions:
- While the scrollbar "thumb" looks pressed when clicked it jumps up again
  when dragged. Not sure where to change this behavior.
- I don't know how to check the @media print section? Where does it take
  effect? I don't remember ever printing something with a visible scrollbar...
- The scrollbars in print preview are currently all white. Where do they
  get their properties from?
- It is also still missing the characteristic handle in its middle. I tried
  the following CSS lines but they didn't have any effect (I also tried to
  add the background images to thumb>gripper but that didn't work, either):

thumb {
  -moz-appearance: scrollbarthumb-vertical;
  background-image: url("skin/classic/global/arrow/thumb-vert.png");
  min-height: 17px;
}
thumb[orient="horizontal"] {
  -moz-appearance: scrollbarthumb-horizontal;
  background-image: url("skin/classic/global/arrow/thumb-horiz.png");
  min-width: 17px;
}

Comment 3

[Peter Weilbacher]

Attachment: ZIP file with used icons

Comment 4

[Peter Weilbacher]

Comment on attachment 221206 [details] [diff] [review]
CSS patch

Gavin, as you CC'd yourself, would you mind to having a first look at the patch?

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 221206 [details] [diff] [review]
CSS patch

I don't have an OS/2 build to test this with, and since that's pretty much all there is to do when reviewing theme changes, I can't review this :).

Comment 6

[Peter Weilbacher]

Comment on attachment 221206 [details] [diff] [review]
CSS patch

Well, so it will be Mike then.

Gavin, even if you don't want to make a real review can you shed any light on the issues I listed in comment 2? I don't think they are OS/2 specific...

Comment 7

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
Could point 2 be related to point 3? I would guess that print preview uses the @media print rules. I don't know about the other issues.

Comment 8

[Peter Weilbacher]

Attachment: Complete fix including a typo correction [checked into trunk on 2006-05-09]

Better use ThreeDShadow instead of ThreeDLightShadow for the upper edge of the scrollbar, so that the horizontal one is clearly seperated.

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 221276 [details] [diff] [review]
Complete fix including a typo correction [checked into trunk on 2006-05-09]

r=mkaply

Comment 10

[Mike Kaply [:mkaply]]

Fix checked in.

Comment 11

[Peter Weilbacher]

Mike, thanks for the quick checkin. Is it possible, though, that the checkin of the PNGs was garbled somehow? As Walter Meinl just noted on the newsgroup checking them out again on OS/2 gives you unusable files. Did you not mark them as binaries (although the CVS server should be smart enough)?

I reopen, also because I think I should try to address the questions from comment 2 before forgetting about this stuff.

Comment 12

[Mike Kaply [:mkaply]]

I'll check the PNGs. Also, I noticed that I get CSS errors on my console on the 50% values in scrollbars.css.

Comment 13

[Mike Kaply [:mkaply]]

PNGs are fixed now. You are correct - they weren't binary.

Comment 14

[Peter Weilbacher]

(In reply to comment #12)
> I'll check the PNGs. Also, I noticed that I get CSS errors on my console on the
> 50% values in scrollbars.css.

Oh yes. That looks like a cut-and-paste error.

Comment 15

[Peter Weilbacher]

Attachment: Updates [checked into trunk]

Seems like the previous patch(es) contained rather many mistakes, most of which I correct in this one:

- the 50% cut-and-paste error
- missing plusses in the jar.mn file
- all graphics -- I think -- are supposed to have the same name
  as in winstripe so that we don't end up with two versions in the
  JAR (I therefore converted the PNG files to GIF and changed the
  entries in the file accordinly)
- to have the scrollbar thumb appear pressed down :active is the
  correct selector (:hover:active isn't)
- "-moz-appearance" is not effective for most things on OS/2
- I also added graphics for the middle of the scrollbar thumb
- Finally, I also finetuned padding and margins in the menubar
  and between menuitems and around the menuseparators so that it
  really is 1:1 as OS/2 native menus. (As remarked on the newsgroup
  the density of menuitems was too high previously)

The only thing that I could not solve is the issue with the scrollbars in print preview. I suggest to postpone that to another bug.

Comment 16

[Mike Kaply [:mkaply]]

Comment on attachment 225780 [details] [diff] [review]
Updates [checked into trunk]

r=mkaply

Comment 17

[Peter Weilbacher]

Comment on attachment 225780 [details] [diff] [review]
Updates [checked into trunk]

OK, patch checked in. I added the new GIF files and removed the now obsolete PNGs.

Comment 18

[Peter Weilbacher]

OK, checked the current trunk stage into branch. I hope I didn't miss any files...

Comment 19

[Peter Weilbacher]

Similar to SeaMonkey in bug 342695 a minor bug in these fixes was discovered in the newsgroup. Instead of filing a new bug I reopen this one for it.

Comment 20

[Peter Weilbacher]

Attachment: Fix for colored menubar without grey gaps and printer panel dropdown [checked into trunk and 1.8 branch]

OK, same fix as for SeaMonkey. (The third fix for the autocomplete dropdown is not needed for toolkit.) I hope this fixes it without introducing other problems (I haven't found any but I use Firefox much less than SeaMonkey).

Comment 21

[Mike Kaply [:mkaply]]

Comment on attachment 232306 [details] [diff] [review]
Fix for colored menubar without grey gaps and printer panel dropdown [checked into trunk and 1.8 branch]

> I hope this fixes it without introducing other problems

That's why we have testers :)

r=mkaply

Comment 22

[Peter Weilbacher]

Comment on attachment 232306 [details] [diff] [review]
Fix for colored menubar without grey gaps and printer panel dropdown [checked into trunk and 1.8 branch]

Minor OS/2 only correction, tested on trunk.

Comment 23

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 232306 [details] [diff] [review]
Fix for colored menubar without grey gaps and printer panel dropdown [checked into trunk and 1.8 branch]

a=drivers for the 181 branch