655691 - Use native button appearance for colorpicker buttons

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[alexander.wilms]

User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Thunderbird/3.1.10

Also see: http://ubuntuone.com/p/s0E/

The buttons to choose the color for text and background in the settings window don't follow the Ambiance theme. I think it is the same issue as in this bug, which has been patched already: https://bugzilla.mozilla.org/show_bug.cgi?id=638481

Reproducible: Always

Steps to Reproduce:
1. Open settings window
2. Switch to "View" pane
3. Switch to "Create" pane

Actual Results:  
Some buttons don't follow the gtk-theme

Expected Results:  
They should look like all the other buttons

Comment 1

[alexander.wilms]

The problems is also existent in Mozilla/5.0 (X11; Linux i686; rv:5.0a2) Gecko/20110505 Thunderbird/3.3a4pre

Comment 2

[Richard Marti (:Paenglab)]

This is not only a Thunderbird issue. Firefox has the same in his preferences and both are using the same code.

-> moving to Core

Comment 3

[Richard Marti (:Paenglab)]

Attachment: Make the colorpicker a normal button

This patch gives the colorpicker a button appearance.

This is my first patch for core. So please, if I took the wrong reviewer, then assign the right one.

Comment 4

[Richard Marti (:Paenglab)]

Attachment: Colorpicker comparison

Image showing the difference between before and after.

Comment 5

[Dão Gottwald [:dao]]

Why aren't you modifying toolkit/themes/winstripe/global/colorpicker.css?

Comment 6

[Richard Marti (:Paenglab)]

Attachment: Buttons under Linux and Win7

I didn't knew, if this is appreciated.
A problem is, Windows (i checked under W7) is using 3px borders and Linux mostly 6px borders for buttons. Windows needs padding and Linux not. With a compromise of padding: 2px (1px is to small for Win) you see the tiny color box under Linux. A solution could be to change the height from 24px to 26px. What o you mean?

Comment 7

[alexander.wilms]

Could you change the button height to 26px and post another screenshot, please?

Comment 8

[Richard Marti (:Paenglab)]

Attachment: Make the colorpicker a normal button

Patch also for Windows with button height of 26px.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: Buttons under Linux and Win7 with 26px height

Comment 10

[alexander.wilms]

Looking good

Comment 11

[Dão Gottwald [:dao]]

How does this look with the 'classic' windows theme?

Comment 12

[Richard Marti (:Paenglab)]

Attachment: Buttons under W7 Classic

Colorpicker under Win7 Classic. Actually I have no XP at hand and can't make screenshots under XP.

Comment 13

[Dão Gottwald [:dao]]

Ok... since we don't want the padding on Linux, I think you should both update winstripe's colorpicker.css and add a gnomestripe copy.

Comment 14

[Richard Marti (:Paenglab)]

Attachment: Patch for Linux and Windows

Leaved the padding: 3px on Windows and under Linux no padding set.

Additionally removed some white-space on .colorpickertile. It was the only selector with this formatting in this file.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 532506 [details] [diff] [review]
Patch for Linux and Windows

>+.colorpickertile[selected="true"] {
>+  border : 2px outset #C0C0C0;
>+
>+}

Please remove the blank line and the space before the colon. Thanks!

Comment 16

[Richard Marti (:Paenglab)]

Attachment: Patch for check-in

Fixing nit from review.

Carrying over r+ from previous patch.

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 532508 [details] [diff] [review]
Patch for check-in

>+.colorpickertile[selected="true"] {
>+  border: 2px outset #C0C0C0;
>+
>+}

The blank line is still there.

Comment 18

[Richard Marti (:Paenglab)]

Attachment: Patch for check-in

Sorry, I saw the space between border and :, but oversaw the empty line.

Comment 19

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/12ac80dfdacb