Draw dropdowns with NSPopupButtonCell

VERIFIED FIXED in mozilla1.9.2a1

Status

()

Core
Widget: Cocoa
P3
normal
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: cbarrett, Assigned: mstange)

Tracking

(Depends on: 1 bug, {verified1.9.1})

Trunk
mozilla1.9.2a1
All
Mac OS X
verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
wanted1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Reporter)

Description

11 years ago
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.
(Reporter)

Updated

11 years ago
Blocks: 54488
No longer blocks: 394092

Updated

11 years ago
Blocks: 394092
No longer blocks: 54488
Flags: blocking1.9+

Updated

11 years ago
Target Milestone: --- → mozilla1.9 M10
(Reporter)

Comment 1

11 years ago
Created attachment 285036 [details] [diff] [review]
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.

Updated

10 years ago
Priority: -- → P4

Updated

10 years ago
Target Milestone: mozilla1.9 M10 → ---
(Reporter)

Updated

10 years ago
Attachment #285036 - Attachment is obsolete: true
(Reporter)

Comment 2

10 years ago
Created attachment 290718 [details] [diff] [review]
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.
Attachment #290718 - Flags: review?(joshmoz)
(Reporter)

Comment 3

10 years ago
Created attachment 291916 [details] [diff] [review]
wip fix 0.8

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

Updated

10 years ago
Assignee: cbarrett → joshmoz

Updated

10 years ago
Priority: P4 → P3

Updated

10 years ago
Blocks: 54488
No longer blocks: 394092

Updated

10 years ago
Attachment #290718 - Flags: review?(joshmoz)

Comment 4

10 years ago
probably not going to make it for 1.9, tough call but we're fast running out of time
Flags: wanted1.9+
Flags: blocking1.9-
Flags: blocking1.9+
Hardware: PC → All
(Assignee)

Updated

9 years ago
Depends on: 465069
(Assignee)

Comment 5

9 years ago
Created attachment 357336 [details] [diff] [review]
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.
Assignee: joshmoz → mstange
Attachment #290718 - Attachment is obsolete: true
Attachment #291916 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #357336 - Flags: superreview?(roc)
Attachment #357336 - Flags: review?(joshmoz)
(Assignee)

Comment 6

9 years ago
Created attachment 357337 [details]
Screenshot: Bugzilla dropdowns before / after, active and inactive window
(Assignee)

Comment 7

9 years ago
Created attachment 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.
(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.
(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.
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.
(Assignee)

Comment 11

9 years ago
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.
Attachment #357336 - Flags: superreview?(roc) → superreview+

Comment 12

9 years ago
Comment on attachment 357336 [details] [diff] [review]
v2

Please file a bug on the degraded look at larger heights when this lands.
Attachment #357336 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 13

9 years ago
I've filed bug 476277.

Pushed: http://hg.mozilla.org/mozilla-central/rev/ee1c68810375
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Depends on: 476277
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
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?
Status: RESOLVED → VERIFIED
(Assignee)

Comment 15

9 years ago
I think we should get it for 1.9.1, but let's let it bake for a week.
Whiteboard: [needs baking on trunk]
(Assignee)

Updated

9 years ago
Attachment #357336 - Flags: approval1.9.1?
Depends on: 480265
There is one more side-effect by this patch. Text for up-scaled dropdowns is overlapping the buttons. See bug 480265.
(Assignee)

Updated

9 years ago
Whiteboard: [needs baking on trunk] → [needs 1.9.1 approval]
(Assignee)

Updated

9 years ago
Duplicate of this bug: 483214
Comment on attachment 357336 [details] [diff] [review]
v2

a191=beltzner
Attachment #357336 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Comment 19

9 years ago
Created attachment 368122 [details] [diff] [review]
1.9.1 branch patch

(only trivial merging)
(Assignee)

Comment 20

9 years ago
pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/305e88a5f35c
Keywords: fixed1.9.1
Whiteboard: [needs 1.9.1 approval]
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
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.