399030 - Draw dropdowns with NSPopupButtonCell

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Colin Barrett [:cbarrett]]

For similar reasons to bug 394892, we want to do this. Additionally, it allows us to draw these widgets without double buffering (they do not render correctly on 10.4 with HITheme), which will result in nicer looking drawing especially for zoomed in pages.

Comment 1

[Colin Barrett [:cbarrett]]

Attachment: fix 0.7

Just storing this here for safe keeping. It should be mostly done except it isn't drawing in its frame correctly, last time I checked.

Comment 2

[Colin Barrett [:cbarrett]]

Attachment: prefix 1.0

The first change makes us closer to Safari in terms of rendering. Baseline was the distance from the right edge of the "h" in Macintosh to the left edge of the blue area in the "Hardware" dropdown on a buzilla bug.

The second change reduces the height of popups by one, which brings us in line with Safari (and standard system controls).

Ran reftests, they passed.

Comment 3

[Colin Barrett [:cbarrett]]

Attachment: wip fix 0.8

Just putting this in bugzilla for archival purposes. It's got changes for bug 394892 as well.

Comment 4

[Josh Aas]

probably not going to make it for 1.9, tough call but we're fast running out of time

Comment 5

[Markus Stange [:mstange]]

Attachment: v2

This patch results in several positive changes:

 - <select>s at the default size no longer look blurry / muddy. Instead, the
   natural small size is used, so it looks crisp (like in Safari).
   Josh, let me know if you want me to reduce the snapping tolerance to 1px
   (like we did for buttons in bug 468507) - however, I don't think that's
   necessary; I haven't seen any problems with the snapping. I've tested a lot
   of sites and overall it looks much better to me.

 - The background window state is correct now. (I'll attach a screenshot)

 - A lot of code is removed.

The only downside with this patch is the fact that dropdowns with a large height don't look as good because we have to scale up the regular size manually.

Comment 6

[Markus Stange [:mstange]]

Attachment: Screenshot: Bugzilla dropdowns before / after, active and inactive window

Comment 7

[Markus Stange [:mstange]]

Attachment: Screenshot: Stretched dropdown on https://addons.mozilla.org/

This is a case where the new dropdowns don't look as good as the old ones.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #7)
> Created an attachment (id=357339) [details]
> Screenshot: Stretched dropdown on https://addons.mozilla.org/
> 
> This is a case where the new dropdowns don't look as good as the old ones.

CC'ing Fred, so he is aware of that and probably has an idea.

Comment 9

[Samuel Sidler (old account; do not CC)]

(In reply to comment #8)
> CC'ing Fred, so he is aware of that and probably has an idea.

Which may fix an AMO case, but won't fix the overall problem. The patch is definitely an improvement for most selects though.

Comment 10

[Fred Wenzel [:wenzel]]

Yeah I can see the problem but I am not sure if it's a good idea to just fix it on the AMO side of things. Fixing it on the client side it he right thing to do here, I would say.

Comment 11

[Markus Stange [:mstange]]

The problem is that Cocoa doesn't give us the option to draw dropdowns of that height. We can't really do much here, other than maybe falling back to the old way of drawing them when they're too high... but then the inactive window state is wrong again, and we get code duplication.

Comment 12

[Josh Aas]

Comment on attachment 357336 [details] [diff] [review]
v2

Please file a bug on the degraded look at larger heights when this lands.

Comment 13

[Markus Stange [:mstange]]

I've filed bug 476277.

Pushed: http://hg.mozilla.org/mozilla-central/rev/ee1c68810375

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Wow, that looks fantastic on trunk. Thanks Markus! Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090201 Minefield/3.2a1pre ID:20090201020604

Markus, does something hinder us to get this into 1.9.1?

Comment 15

[Markus Stange [:mstange]]

I think we should get it for 1.9.1, but let's let it bake for a week.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

There is one more side-effect by this patch. Text for up-scaled dropdowns is overlapping the buttons. See bug 480265.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 357336 [details] [diff] [review]
v2

a191=beltzner

Comment 19

[Markus Stange [:mstange]]

Attachment: 1.9.1 branch patch

(only trivial merging)

Comment 20

[Markus Stange [:mstange]]

pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/305e88a5f35c

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified on 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090402 Shiretoko/3.5b4pre ID:20090402031241