-moz-appearance: menulist-button broken in GTK

RESOLVED FIXED in mozilla1.9beta1

Status

()

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

People

(Reporter: pantsgolem, Assigned: Michael Ventnor)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9a6pre) Gecko/20070604 Minefield/3.0a6pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9a6pre) Gecko/20070605 Minefield/3.0a6pre

Between 20070604 and 20070605, -moz-appearance: menulist-button (used, for example, in themes with a "native" location bar) stopped working in GTK.  Works fine in Windows.

Reproducible: Always

Steps to Reproduce:
1. Enable the default theme
2. Put ".autocomplete-history-dropmarker { -moz-appearance: menulist-button !important; }" in userChrome.css
3. Start firefox
Actual Results:  
The autocomplete dropdown button appears in the default theme's style.

Expected Results:  
The dropdown button should be an approximation of a native GTK widget.

The timing of this bug coincides with the initial checkin for bug 329846.

Incidentally, is there a bug for improving the appearance of menulist-button on GTK?

Updated

11 years ago
Blocks: 329846
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
(Assignee)

Comment 1

11 years ago
I commented out dropdown buttons because they look uuuug-ly when they're used with a combobox that has styles applied to it. 

They're not really native anyway, they're just regular buttons with the downward scroll button arrow applied on it. I could turn it back on but I'd prefer not to, since those buttons stick out like a sore thumb.
(Reporter)

Comment 2

11 years ago
I have three problems with that:

1. Commenting out that line disabled the pseudo-native dropdown in chrome as well as content, and menulist-button is necessary for themes that aim for a native addressbar.
2. Enabling the pseudo-native dropdown in content is consistent with the behavior on Windows.
3. If the dropdown button is enabled in source, users are free to override in CSS.  By disabling it in source, it's disabled for everybody.
(Assignee)

Comment 3

11 years ago
Created attachment 280830 [details] [diff] [review]
Patch

This turns it back on, because I found what I wanted to style the button more natively than what we did before (which was just a normal button).
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
Attachment #280830 - Flags: superreview?(roc)
Attachment #280830 - Flags: review?(roc)
(Assignee)

Comment 4

11 years ago
Oh, and the reason for this:

-      // for dropdown textfields, look at the parent frame (textbox or menulist)
-      if (aWidgetType == NS_THEME_DROPDOWN_TEXTFIELD)
-        aFrame = aFrame->GetParent();

is that the textbox wouldn't repaint after it lost focus. It would still glow even if another textbox had focus.
Attachment #280830 - Flags: superreview?(roc)
Attachment #280830 - Flags: superreview+
Attachment #280830 - Flags: review?(roc)
Attachment #280830 - Flags: review+
Attachment #280830 - Flags: approval1.9+
(Assignee)

Updated

11 years ago
Keywords: checkin-needed
Checking in widget/src/gtk2/gtk2drawing.c;
/cvsroot/mozilla/widget/src/gtk2/gtk2drawing.c,v  <--  gtk2drawing.c
new revision: 1.30; previous revision: 1.29
done
Checking in widget/src/gtk2/nsNativeThemeGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v  <--  nsNativeThemeGTK.cpp
new revision: 1.103; previous revision: 1.102
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M9
could this have caused bug 396299?
(Reporter)

Comment 7

11 years ago
Created attachment 281063 [details]
screenshot: no arrow, wrong height

This is what styled <select> looks like for me now.  There's no arrow and the button does not extend to the bottom of the box.

This was with gtk 2.11.6 (and now with 2.12.0) so I wouldn't be surprised if something there changed.
(Assignee)

Comment 8

11 years ago
What theme is that? Clearlooks?
BTW it WFM on GTK 2.10.0
(Assignee)

Comment 9

11 years ago
I've been tricked. It seems the native entry button isn't flexible in its size at all, it just so happens that most styled dropdowns do fit rather nicely. Thats why you're seeing the space.

As for the arrow, I can't really explain that. I'll have to look into it.

Bah, can someone backout this patch and reopen the bug?
(Reporter)

Comment 10

11 years ago
That particular theme is QtCurve, but I've tried Clearlooks and Xfce as well, and it's the same.
Patch backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 12

11 years ago
Created attachment 284587 [details] [diff] [review]
Patch 2

This restores our old buttons, but only for XUL which was mostly affected by the regression. Under HTML the "regression" is actually the better scenario, the GTK buttons have the wrong size and shape and they don't fit in at all with the generic dropdown's border.

I think this is the best compromise.
Attachment #280830 - Attachment is obsolete: true
Attachment #284587 - Flags: superreview?(roc)
Attachment #284587 - Flags: review?(roc)
Comment on attachment 284587 [details] [diff] [review]
Patch 2

+    if (aFrame)
+      return aFrame->GetContent()->IsNodeOfType(nsINode::eXUL) &&
+             !IsWidgetStyled(aPresContext, aFrame, aWidgetType);
+    else
+      return !IsWidgetStyled(aPresContext, aFrame, aWidgetType);

Simplify this to
  (!aFrame || aFrame->GetContent()->IsNodeOfType(nsINode::eXUL)) &&
  !IsWidgetStyled(aPresContext, aFrame, aWidgetType)
Attachment #284587 - Flags: superreview?(roc)
Attachment #284587 - Flags: superreview+
Attachment #284587 - Flags: review?(roc)
Attachment #284587 - Flags: review+
(Assignee)

Comment 14

11 years ago
Created attachment 284601 [details] [diff] [review]
Simplified

This is a very trivial patch that fixes a noticeable regression with dropdown buttons since the landing of native themes.
Attachment #284587 - Attachment is obsolete: true
Attachment #284601 - Flags: approval1.9?
Attachment #284601 - Flags: approvalM9?
(Assignee)

Comment 15

11 years ago
Although not critical, this patch would be a good idea for Beta 1 so people can confidently use this button in their themes on Linux. This patch is also quite small and safe.
Comment on attachment 284601 [details] [diff] [review]
Simplified

a=endgame drivers for M9
Attachment #284601 - Flags: approvalM9?
Attachment #284601 - Flags: approvalM9+
Attachment #284601 - Flags: approval1.9?
Attachment #284601 - Flags: approval1.9+
Checking in widget/src/gtk2/nsNativeThemeGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v  <--  nsNativeThemeGTK.cpp
new revision: 1.106; previous revision: 1.105
done
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.