188071 - Missing style entity for default appearance of buttons

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Mike Kaply [:mkaply]]

I'm not sure where this will end up, but I thought I would get it opened.

When you highlight a XUL button in Mozilla, it gets a black box around it - the
default box.

I did some investigation and found that this box is drawn with the 3D Dark
Shadow color from nsLookAndFeel.

This is not correct.

There should be a new Moz color for the default around push buttons. It should
not be coded to be the same as the 3D Dark Shadow.

So first we need a new CSS entity for specifying the default rectangle around
buttons.

Comment 1

[Mike Kaply [:mkaply]]

*** Bug 208655 has been marked as a duplicate of this bug. ***

Comment 2

[Jessica Blanco]

Attachment: Added a default button color entity

Added buttondefault to all platform directories in
widget/src/xx/nsLookAndFeel.cpp.  I kept the color that is currently being used
for threeddarkshadow for buttondefault except for OS/2. Browser buttons will
have same look. The value returned for buttondefault can be modified to be
different than the value set now.  I also changed button.css in
themes/classic/global/win. I modified the button colors to use buttondefault as
the button border instead of threeddarkshadow. I only modified this file in the
win directory so that OS/2 will use these changes. Other platforms may also
make these changes. Other files could also get changed later to use the
buttondefault color instead of threeddarkshadow.

Comment 3

[Mike Kaply [:mkaply]]

Comment on attachment 125592 [details] [diff] [review]
Added a default button color entity

There's some tabbing issues in mac/nsLookAndFeel, but overall looks good.

Just to be clear for sr, on all platforms buttondefault is set to the same as
3Ddark (shadow)

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 125592 [details] [diff] [review]
Added a default button color entity

>Index: content/shared/public/nsCSSKeywordList.h
>===================================================================
>RCS file: /cvsroot/mozilla/content/shared/public/nsCSSKeywordList.h,v
>retrieving revision 3.48
>diff -u -r3.48 nsCSSKeywordList.h
>--- content/shared/public/nsCSSKeywordList.h	2 May 2003 22:23:19 -0000	3.48
>+++ content/shared/public/nsCSSKeywordList.h	13 Jun 2003 16:31:54 -0000
>@@ -192,6 +192,7 @@
> CSS_KEY(bottom, bottom)
> CSS_KEY(bounding-box, bounding_box)
> CSS_KEY(button, button)
>+CSS_KEY(buttondefault, buttondefault)

This needs to be -moz-buttondefault, _moz_buttondefault (with similar changes
throughout), since this isn't a keyword defined in any CSS spec.

>+  -moz-border-top-colors: ButtonDefault ThreeDHighlight transparent;

For things like this, I'd prefer the captitalization -moz-ButtonDefault to be
consistent with what we do for -moz-Dialog, etc.

>Index: widget/public/nsILookAndFeel.h
>@@ -81,6 +81,7 @@
>     eColor_activecaption,
>     eColor_appworkspace,
>     eColor_background,
>+    eColor_buttondefault,
>     eColor_buttonface,
>     eColor_buttonhighlight,
>     eColor_buttonshadow,

This should also use eColor__moz_buttondefault.

>Index: widget/src/mac/nsLookAndFeel.cpp
>@@ -193,6 +193,9 @@
>         aColor = NS_RGB(0x63,0x63,0xCE);
>         res = NS_OK;
>         break;
>+    case eColor_buttondefault:
>+		res = GetMacBrushColor(kThemeBrushButtonActiveDarkShadow, aColor, NS_RGB(0x77,0x77,0x77));
>+	    break;
>     case eColor_buttonface:
>     	res = GetMacBrushColor(kThemeBrushButtonFaceActive, aColor, NS_RGB(0xDD,0xDD,0xDD));
> 	    break;

Watch the tabs!

>Index: widget/src/xpwidgets/nsXPLookAndFeel.cpp
>+  "ui.buttondefault",

and this pref also needs a -moz-.

Comment 5

[Jessica Blanco]

Attachment: above patch with -moz-buttondefault changes

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 125598 [details] [diff] [review]
above patch with -moz-buttondefault changes

>Index: widget/public/nsILookAndFeel.h
>@@ -129,7 +129,8 @@
>     eColor__moz_mac_accentdarkestshadow,
>   
>     // keep this one last, please
>-    eColor_LAST_COLOR
>+    eColor_LAST_COLOR,
>+    eColor__moz_buttondefault
>   } nsColorID;

Don't put it after LAST_COLOR (and put it where it agrees with your change to
sColorPrefs in nsXPLookAndFeel.cpp).

>Index: widget/src/mac/nsLookAndFeel.cpp
>@@ -324,6 +324,9 @@
>     	//get this colour by querying variation table, ows. default to Platinum/Lavendar
> 		res = GetMacAccentColor(eColorOffset_mac_accentdarkestshadow, aColor, NS_RGB(0x00,0x00,0x55));
> 	    break;
>+    case eColor__moz_buttondefault:
>+          res = GetMacBrushColor(kThemeBrushButtonActiveDarkShadow, aColor, NS_RGB(0x77,0x77,0x77));
>+         break;
>     default:
>         NS_WARNING("Someone asked nsILookAndFeel for a color I don't know about");
>         aColor = NS_RGB(0xff,0xff,0xff);

There's still a tab hiding in there.

>Index: widget/src/photon/nsLookAndFeel.cpp
>@@ -212,6 +212,10 @@
> 	  aColor = PH_TO_NS_RGB(Pg_LGREY);
> 	  break;
> 
>+     case eColor__moz_buttondefault:
>+       aColor = PH_TO_NS_RGB(Pg_DGREY);
>+       break;
>+
>   	default:
>     aColor = PH_TO_NS_RGB(Pg_WHITE);
>     break;

If you're going to convert to spaces here, do it according to the modeline
(which says tabs are a 4-space indent -- and that's consistent with the rest of
the file).

With those things fixed, sr=dbaron.

Comment 7

[Jessica Blanco]

Attachment: updated patch

I placed -moz-buttondefault in the same place in nsILookAndFeel as in
nsXPLookAndFeel. It doesn't belong under CSS2 or CSS3 so placed it right above
the CSS3 comment apart from the CSS2 group.

Comment 8

[Hixie (not reading bugmail)]

This should not be added to -moz-appearance. The appearance property should work
out that it is painting a default button and render itself appropriately, just
like it does for the other button states (focussed, checked, pressed, disabled,
and so forth).

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This *isn't* -moz-appearance.  It's system colors.

Comment 10

[Hixie (not reading bugmail)]

Oh, my bad. I was confused by the 'button-default' I just argued out of CSS3 UI.
Carry on, nothing to see here. :-)

Comment 11

[Mike Kaply [:mkaply]]

All bits checked in.