additional margin around menu separator items

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla52
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(3 attachments)

This adds spacing around separators inconsistent with native apps, and gaps between any backgrounds drawn in the menu items (to address bug 1287036).
Comment on attachment 8808479 [details]
bug 1315873 remove unused non-native fallback menuseparator styling, including margin accidentally applied to native separators

Can you use padding instead of border-top/bottom, set a height, use background-origin: content-box and get rid of the transparent portions of the gradient?

Also the following rule (menulist > menupopup > menuseparator) needs an update.
Attachment #8808479 - Flags: review?(dao+bmo) → review-
(In reply to Dão Gottwald [:dao] from comment #4)
> Can you use padding instead of border-top/bottom, set a height, use
> background-origin: content-box and get rid of the transparent portions of
> the gradient?

If a height is set here, then that will be applied even with non-none -moz-appearance.  That's not ideal, but provided native menus are never smaller than this height, then perhaps zero padding and minimum size could be provided instead of a border size for native menuseparators.  I'll have to check this as (?xul) layout ignores minimum sizes for native widgets in some situations and I don't know the reasons for this or which situations are affected.

Would you prefer this solution?
It may be a little more complicated in the native widget code, but not crazy.
The main disadvantage is the height behaving as a minimum size even for native widgets.

Can you explain your concerns with background-origin:border-box and/or transparent gradient to help me understand, please?

> Also the following rule (menulist > menupopup > menuseparator) needs an
> update.

Thanks.  I will look at that.
Flags: needinfo?(dao+bmo)
(In reply to Karl Tomlinson (:karlt) from comment #5)
> Can you explain your concerns with background-origin:border-box and/or
> transparent gradient to help me understand, please?

It just seemed unnecessarily complicated to me.

I think we can also just remove the fallback styling. toolkit/themes/osx/global/menu.css doesn't provide this for instance.
Flags: needinfo?(dao+bmo)
(In reply to Dão Gottwald [:dao] from comment #6)
> I think we can also just remove the fallback styling.
> toolkit/themes/osx/global/menu.css doesn't provide this for instance.

Oh, I had assumed they must be used somewhere, but panelUI and
xul|menulist are the only overrides mentioning menuseparator explicitly, and
they both override all of border-top/bottom, margin, and padding.

http://searchfox.org/mozilla-central/rev/008fdd93290c83325de6629a1ae48dc2afaed868/browser/themes/shared/customizableui/panelUI.inc.css#1181
http://searchfox.org/mozilla-central/rev/008fdd93290c83325de6629a1ae48dc2afaed868/toolkit/themes/shared/in-content/common.inc.css#421

So that seems the perfect solution, thanks.
Attachment #8808479 - Flags: review?(dao+bmo) → review+
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db092219b871
remove unused non-native fallback menuseparator styling, including margin accidentally applied to native separators r=dao
https://hg.mozilla.org/mozilla-central/rev/db092219b871
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.