856040 - Adjust padding of XUL menulist to match native widget on Windows 7 and up

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[Richard Marti (:Paenglab)]

From Bug 853431 comment 7:
While this fixes color issues, it doesn't fix the padding issue:
On Windows 7 and up (and probably Vista), our menulist widget (the thing you click to open the menu, not the menu itself) has 3 pixels of extra vertical padding (2px below and 1 above, but that's probably just pixel-snapped centering) that the native menulist does not have.

Comment 1

[Frank Yan (:fryn)]

Thanks for filing this. :)

Comment 2

[Richard Marti (:Paenglab)]

Attachment: proposed fix

This patch makes the menulist widget paddings mostly equal to the native widget. I let a 1px gap on the left of the text to the focus ring. The native widget has no gap but I think this doesn't look good.

This patch needs the patch from Bug 853431 applied first.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: comparison between native and toolkit menulist under Win7 and Win8

Comment 4

[Frank Yan (:fryn)]

Comment on attachment 731555 [details] [diff] [review]
proposed fix

Review of attachment 731555 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch on the redundant styles in menulist.css.
Just one small issue when running with a HiDPI display:

::: toolkit/themes/windows/global/menulist-aero.css
@@ +21,5 @@
> +    -moz-margin-start: 0 !important;
> +  }
> +
> +  .menulist-dropmarker {
> +    margin-top: -3px;

At least on my machine (running Windows 8 with a HiDPI display), this looks better when margin-top is -2px, maybe due to pixel snapping. How does it look with -2px to you?

Comment 5

[Richard Marti (:Paenglab)]

Attachment: proposed fix v2

You're right, I don't know but before -3px was correct and now -2px is better. Maybe it was a rule left during testing.

Comment 6

[Frank Yan (:fryn)]

Comment on attachment 731790 [details] [diff] [review]
proposed fix v2

Thanks for confirming and for working on this. :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/96a67be7802b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/96a67be7802b