GTK buttons should have a "default button" style if its the default button

RESOLVED FIXED in mozilla1.9alpha8

Status

()

Core
Widget: Gtk
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Michael Ventnor, Assigned: Michael Ventnor)

Tracking

Trunk
mozilla1.9alpha8
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

4.03 KB, patch
roc
: review+
Stuart Parmenter
: approval1.9+
Details | Diff | Splinter Review
(Assignee)

Description

10 years ago
 
(Assignee)

Updated

10 years ago
Summary: GTK buttons should have a "default button → GTK buttons should have a "default button" style if its the default button
(Assignee)

Comment 1

10 years ago
Created attachment 278266 [details] [diff] [review]
Patch

Whoops, submitted this bug accidentally and too early.

On other platforms we support giving default buttons (the button activated when you hit Return, eg. the OK button in dialogs) a special styling. On GTK, there is commented-out code for this, but the code is wrong.

We can apply the GTK theme's "default button" style by setting one flag, so I imagine this patch is trivial enough for 1.9.
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
Attachment #278266 - Flags: superreview?(roc)
Attachment #278266 - Flags: review?(roc)
(Assignee)

Updated

10 years ago
Blocks: 329846
+    if (state->isDefault)
+        GTK_WIDGET_UNSET_FLAGS(widget, GTK_HAS_DEFAULT);

Could this be unconditional? I'm assuming GTK_WIDGET_UNSET_FLAGS is very cheap.
(Assignee)

Comment 3

10 years ago
Would it be cheaper than an if() statement though? I only put it there because I'm  not sure whether that macro will cause problems if the flag isn't set anyway.
It doesn't have to be cheaper than an if() statement, it just has to be similar ... then removing one line of code would be a good thing.

> I only put it there because I'm  not sure whether that macro will cause
> problems if the flag isn't set anyway.

It won't:

#define GTK_WIDGET_UNSET_FLAGS(wid,flag)  G_STMT_START{ (GTK_WIDGET_FLAGS (wid) &= ~(flag)); }G_STMT_END
(Assignee)

Comment 5

10 years ago
Created attachment 278382 [details] [diff] [review]
Patch 2
Attachment #278266 - Attachment is obsolete: true
Attachment #278382 - Flags: superreview?(roc)
Attachment #278382 - Flags: review?(roc)
Attachment #278266 - Flags: superreview?(roc)
Attachment #278266 - Flags: review?(roc)
Attachment #278382 - Flags: superreview?(roc)
Attachment #278382 - Flags: superreview+
Attachment #278382 - Flags: review?(roc)
Attachment #278382 - Flags: review+
Attachment #278382 - Flags: approval1.9?

Updated

10 years ago
Attachment #278382 - Flags: approval1.9? → approval1.9+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed

Comment 6

10 years ago
mozilla/widget/src/gtk2/gtk2drawing.c         1.29
mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp  1.102
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M8
You need to log in before you can comment on or make changes to this bug.