Button on dropdown lists too skinny

RESOLVED FIXED

Status

()

RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: bugzilla, Unassigned)

Tracking

({regression})

Trunk
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

13 years ago
Gentoo Linux, Gnome, Firefox trunk.

The button on dropdown lists is too skinny. It's been this way for quite a while (I have the regression range narrowed down to "between Feb. 9 and Mar. 11", I'll get it narrowed down more). I'll attach a screenshot to show the problem. 

I won't be surprised if this is a dupe, since it's been here for so long, but I couldn't find it in a search...
(Reporter)

Comment 1

13 years ago
Created attachment 221539 [details]
Screenshot

Here's the screenshot, regression range coming.

Comment 3

13 years ago
Nothing jumps out at me in the check-in range except cairo stuff. ccing vlad.
This has something to do with native theme handling; roc did a large chunk of that code, I'd guess that we're not handling some internal padding or somesuch and giving gtk a too-small rectangle in which to draw the element.

Comment 5

12 years ago
So this may have something to do with theming, but the fact is that nothing theme-related landed during the regression range.

Looking at the check-ins, bug 78081 looks suspicious. There is also bug 332640, which was supposedly backed out (see bug 333051).
Flags: blocking1.9?
Guys, this just regressed when cairo was enabled in nightlies; this presumably wasn't a checkin but a configuration change on the tinderbox; checking about:buildconfig should confirm.

Anyway, the reason this regressed is that the combobox makes the dropdown marker the same width as the dropdown's scrollbar, and the implementation of nsThebesDeviceContext::GetScrollBarDimensions is just completely bogus on Linux.  For that matter, it's bogus on all non-Windows platforms; once Mac cairo is on you'll get this problem there too.
Blocks: 334737
OS: Linux → All
Hardware: PC → All

Comment 7

12 years ago
Created attachment 234541 [details] [diff] [review]
Use what nsDeviceContextGTK was using (for now)

Boris pointed me to where to find this on IRC; I just made the patch. It's sort of a hack, but it works.

He also said that we should possibly move this function into native theme stuff somewhere since we're going to need it for Windows, Linux, Mac, etc.

Comment 8

12 years ago
Comment on attachment 234541 [details] [diff] [review]
Use what nsDeviceContextGTK was using (for now)

It'd make me happy if we could get this in, at least until we come up with a better solution. I miss pretty dropdown buttons. :(
Attachment #234541 - Flags: superreview?(vladimir)
Attachment #234541 - Flags: review?(vladimir)
Comment on attachment 234541 [details] [diff] [review]
Use what nsDeviceContextGTK was using (for now)

I guess this is ok, though I'd prefer to cache this and blow it away when the theme changes, but whatever.
Attachment #234541 - Flags: superreview?(vladimir)
Attachment #234541 - Flags: superreview+
Attachment #234541 - Flags: review?(vladimir)
Attachment #234541 - Flags: review+

Comment 10

12 years ago
OK. I filed bug 349370 for caching the scrollbar size.
Whiteboard: [checkin needed]
Er... we need to cache this, like non-cairo builds do.  Otherwise it'll almost certainly be a performance issue.

Comment 12

12 years ago
Created attachment 235687 [details] [diff] [review]
Use what nsDeviceContextGTK was using (for now) v2.0

OK. This patch caches the size just like nsDeviceContextGTK did.
Attachment #234541 - Attachment is obsolete: true
Attachment #235687 - Flags: superreview?(vladimir)
Attachment #235687 - Flags: review?(vladimir)

Updated

12 years ago
Whiteboard: [checkin needed]

Comment 13

12 years ago
*** Bug 349370 has been marked as a duplicate of this bug. ***
Comment on attachment 235687 [details] [diff] [review]
Use what nsDeviceContextGTK was using (for now) v2.0

ifdef MOZ_ENABLE_GTK2 around the mGTKScrollbarHeight/Width members; they don't need to be there.

Also, please file a new bug for moving scrollbar width/height code into widget native theme code; this code shouldn't live on the device context.
Attachment #235687 - Flags: superreview?(vladimir)
Attachment #235687 - Flags: superreview+
Attachment #235687 - Flags: review?(vladimir)
Attachment #235687 - Flags: review+

Comment 15

12 years ago
Created attachment 236139 [details] [diff] [review]
Use what nsDeviceContextGTK was using (for now) v2.1

OK. I filed bug 350766 to move this code into widget/.
Attachment #235687 - Attachment is obsolete: true
Attachment #236139 - Flags: superreview+
Attachment #236139 - Flags: review+
Checked in
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: blocking1.9?
You need to log in before you can comment on or make changes to this bug.