Closed Bug 254002 Opened 18 years ago Closed 14 years ago

<toolbarbutton> borders not stylable on linux; checking not working

Categories

(Core :: XUL, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 404514

People

(Reporter: alex, Assigned: jag+mozilla)

References

()

Details

Attachments

(1 file, 8 obsolete files)

See testcase: http://www.croczilla.com/~alex/testcases/toolbar-bug.xul
The first toolbar button has attribute checked="true" and should be displayed
with an inset-type border. It is under Windows but not under Linux. 

More generally, styling of borders doesn't work for toolbarbuttons under Linux
at all: The second button should appear with a 2px solid red border. Again, it
doesn't under Linux.
what happens if you also style it with -moz-appearance:none !important; ?
(In reply to comment #1)
> what happens if you also style it with -moz-appearance:none !important; ?

See http://www.croczilla.com/~alex/testcases/toolbar-bug2.xul
Interestingly, this makes the 'checked' case work, but not the non-checked one...
hm... what if you make the border !important, too?
Good point, but doesn't work either unfortunately. I've modified
http://www.croczilla.com/~alex/testcases/toolbar-bug2.xul accordingly.
Assignee: dbaron → jag
Component: Style System (CSS) → XP Toolkit/Widgets
QA Contact: ian → jrgmorrison
Attached patch patch ver 1 (obsolete) — Splinter Review
This patch adds native rendering of checked buttons in gtk2. First testcase is solved by this change, second works for me even without this patch.

Who can review this?
Attachment #207136 - Flags: review?
FYI,

The first testcase gives me two default buttons, no styles what so ever.

The seccond testcase gives me two styled buttons, the first "selected/checked" and The seccond with a 2px(i think) red border all around.

No patches aplied, plain vanilla 1.5;
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

Attached patch ver 2 (obsolete) — Splinter Review
Remove gtk warnings and some indentation fixes.
Attachment #207136 - Attachment is obsolete: true
Attachment #209226 - Flags: review?(bryner)
Attachment #207136 - Flags: review?
Comment on attachment 209226 [details] [diff] [review]
ver 2

>--- .pc/gtk-toolbarbutton-checked-fix.diff/widget/src/gtk2/gtk2drawing.c	2006-01-21 18:44:26.000000000 +0100
>+++ widget/src/gtk2/gtk2drawing.c	2006-01-21 19:01:33.000000000 +0100
>@@ -940,17 +945,17 @@ moz_gtk_dropdown_arrow_paint(GdkDrawable
> {
>     GdkRectangle arrow_rect, real_arrow_rect;
>     GtkStateType state_type = ConvertGtkState(state);
>     GtkShadowType shadow_type = state->active ? GTK_SHADOW_IN : GTK_SHADOW_OUT;
>     GtkStyle* style;
> 
>     ensure_arrow_widget();
>     moz_gtk_button_paint(drawable, rect, cliprect, state, GTK_RELIEF_NORMAL,
>-                         gDropdownButtonWidget);
>+                         gDropdownButtonWidget, 0);

Use FALSE here instead of 0 to be consistent with the gboolean type.

>@@ -1567,17 +1572,18 @@ gint
> moz_gtk_widget_paint(GtkThemeWidgetType widget, GdkDrawable* drawable,
>                      GdkRectangle* rect, GdkRectangle* cliprect,
>                      GtkWidgetState* state, gint flags)
> {
>     switch (widget) {
>     case MOZ_GTK_BUTTON:
>         ensure_button_widget();
>         return moz_gtk_button_paint(drawable, rect, cliprect, state,
>-                                    (GtkReliefStyle) flags, gButtonWidget);
>+                                    (GtkReliefStyle) flags&MOZ_GTK_BUTTON_RELIEF_MASK,
>+                                    gButtonWidget, flags&MOZ_GTK_BUTTON_CHECKED);

Space on both sides of '&'

Looks good otherwise.
Attachment #209226 - Flags: review?(bryner) → review+
Attached patch ver. 3 (obsolete) — Splinter Review
Updated to reflect review comments.
Attachment #209226 - Attachment is obsolete: true
Attachment #210033 - Flags: superreview?(dbaron)
Attachment #210033 - Flags: review+
Comment on attachment 210033 [details] [diff] [review]
ver. 3

>-    GtkStateType button_state = ConvertGtkState(state);
>+    GtkStateType button_state =
>+        (checked && !state->inHover) ? GTK_STATE_ACTIVE : ConvertGtkState(state);

Might you not want to do this if state->disabled ?

sr=dbaron, although this isn't really an area of code I'm particularly strong in
Attachment #210033 - Flags: superreview?(dbaron) → superreview+
Attached patch ver. 4 (obsolete) — Splinter Review
This update fix handling of disabled state buttons. Brian do you wan't to review it again?
Attachment #210033 - Attachment is obsolete: true
Attached patch CSS tweak (obsolete) — Splinter Review
To fully mimmic gtk look small CSS tweak is needed. It changes label color for disabled and checked toolbar buttons.
Attached file New testcase
Updated testcase, which shows also buttons in disabled state.
Attached patch ver. 5 (obsolete) — Splinter Review
Don't add trailing whitespaces...
Attachment #211117 - Attachment is obsolete: true
Attachment #211121 - Flags: superreview+
Attachment #211121 - Flags: review+
Attached patch v6 (obsolete) — Splinter Review
Updated to tip.
Attachment #211119 - Attachment is obsolete: true
Attachment #211121 - Attachment is obsolete: true
Any chances for getting this in for Fx 2.0?
Pawe³, a "version 7" of this patch should probably include reverting the changes caused by the so-called "fix" for bug 334596.
*** Bug 320122 has been marked as a duplicate of this bug. ***
*** Bug 352001 has been marked as a duplicate of this bug. ***
No longer blocks: 352001
Attached patch Patch updated to the trunk (obsolete) — Splinter Review
Updated the patch to the trunk as of 2007-12-27.
Attachment #231952 - Attachment is obsolete: true
Attachment #294663 - Flags: review?(bugz)
Comment on attachment 294663 [details] [diff] [review]
Patch updated to the trunk

Marking this patch obsolete because the trunk as on 2007-12-28 has proper support to this.
Attachment #294663 - Attachment is obsolete: true
Attachment #294663 - Flags: review?(bugz)
Marking it as a duplicate of a much later bug because the later one got committed to CVS.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 404514
You need to log in before you can comment on or make changes to this bug.