Closed Bug 1018595 Opened 10 years ago Closed 10 years ago

Context menu items are inconsistently spaced

Categories

(Firefox :: Theme, defect)

32 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 34

People

(Reporter: nagisa, Assigned: nagisa)

References

Details

Attachments

(5 files, 2 obsolete files)

Attached image The first attachment
In linux with Adwaita theme iconised items are spaced differently compared to all other items.

For example the back button has 4 (or 5 depending on whether you include the border) pixels padding on the left and 4 pixels of padding on the top. Regular menu items have 0px padding and highlight borders with blue as well (see the first attachment)

Moreover, the icons are spaced doubly on the top if there’s items above those icon items. See the second attachment.
Attached image The second attachment (obsolete) —
Summary: Context menu icon items are inconsistently spaced → Context menu items are inconsistently spaced
Blocks: 1000513
Component: Menus → Theme
Comment on attachment 8432103 [details]
The second attachment

There shouldn't be any items above (and we're fixing that in another bug), so I'm marking that screenshot as obsolete for now. We can focus on the other issues you highlighted. :-)
Attachment #8432103 - Attachment is obsolete: true
The first issue will be fixed by bug 1016405.
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
I reopened this issue as bug 1016405 did not fix the issue. Patch attached.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → simonas+bugzilla.mozilla.org
Attachment #8458696 - Flags: review?(dao)
Comment on attachment 8458696 [details] [diff] [review]
Patch v1

I'm afraid this makes the icons look misaligned when they're not hovered, as there's less space above them than below them.
Attachment #8458696 - Flags: review?(dao)
I have 2 counter-arguments to that:

1. Currently context menu looks inconsistent with pretty much any theme unless the theme also has 4 pixel padding on top of context menu. I am not aware of any theme which is bundled with a major distribution and has 4 pixels of padding in that place (though I can’t say I’ve seen many of them either)
2. Following your complaint, in case context menu has only a single *regular* item before a separator, the text on first item should look misaligned too. The attachment is a screenshot of genuine context menu from GTK+ app.

Between what parts of context menu do you want icons to be aligned? Do you want icon to have the same amount of space between (the top of context menu and the top of icon) and (the bottom of icon and the visible part of separator)?

We should go for consistency and not appearance here IMO.
Also hardcoded value of 4 pixels is bad. For example GTK+2 Adwaita theme has 2 pixels margin-top on separator so I could say it looks as misaligned with those 4 pixels as without without them. I bet different themes also have different margins for the separator.
I still think my approach is the correct one. Can I get somebody to ui-review it?
Flags: needinfo?(dao)
If you request ui-review from ux-review@mozilla.com, somebody should theoretically take care of that.
Flags: needinfo?(dao)
Attachment #8458696 - Flags: ui-review?(ux-review)
Could you please attached a screenshot, simukis?
(In reply to Sevaan Franks [:sevaan] from comment #11)
> Could you please attached a screenshot, simukis?

Sure, no problem! I will do screenshots for more themes if requested.

Note: Native screenshots are for GTK+3. The GTK+3 theme has 2 more pixels of margin-top on separators compared to GTK+2 (on which Firefox is currently based) theme.
Comment on attachment 8460309 [details]
Screenshot of context menus

Please also provide a screenshot using Ubuntu.
This is screenshots of context menus made with fresh Ubuntu install for all themes that are available.

I made several mistakes making screenshots for high contrast theme (reload button looks odd, native context menu is different). Please disregard them.
Attachment #8458696 - Flags: review+
Attachment #8458696 - Attachment is obsolete: true
Attachment #8464113 - Flags: ui-review+
Attachment #8464113 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/428c28ca3e36
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 34
QA Whiteboard: [qa+]
Reproduced this issue on
Nightly 32.0a1
Build Id: 20140531030204
User Agent: Mozilla/5.0 (X11; Linux i686; rv:32.0) Gecko/20100101 Firefox 32.0

Verified as fixed on
Firefox 34 Beta 9
Build Id: 20141114133026
User Agent: Mozilla/5.0 (X11; Linux i686; rv:34.0) Gecko/20100101 Firefox 34.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: