614692 - Application menu is not correctly natively themed under Linux

Status: RESOLVED WONTFIX

(Firefox :: Theme, defect)

Description

[Bill Gianopoulos [:WG9s]]

There are several issues here, most of which are caused by the fact that having the Application menu be a single menu pop-up but using XUL to force it to display in 2 columns subverts some of the native theming widgets.  THis results in the following issues:

1.  When you hover over menu items, there should be a 1px gap between the highlight and the border.  This does not occur on the border between the Primary and Secondary panes.

2.  Sub-menus should overlap the parent menu by 1px so that the borders do not overlap.  This also does not occur on the Primary pane because the widget code does not recognize the border between the 2 panes.

3.  Thew Windows code defines the border between the Primary and Secondary panes at the end of the Primary pane, where the Linux code defines it at the start of the Secondary pane.  This is not terribly consistent (although the inconsistency is entirely my fault).

Comment 1

[Bill Gianopoulos [:WG9s]]

Attachment: patch v1

This addresses the above issues.

Comment 2

[Bill Gianopoulos [:WG9s]]

This patch addresses issues 1 and 2 above, but inserting 1ps of padding before the border and 1px of margin after it.

For issue 3, I realized that it would be even easier for third-part themers to, instead of just resolving this inconsistency, to just define the default border in browser/base/content/browser.css so that the default border would appear in third party themes without requiring any coding.  This of course could be over-ridden if desired.

What this patch does NOT address:

1.  This patch does not address the issue raised in bug 613880.  I can not reproduce this issue and it is difficult for me to fix something I can't reproduce.

2.  This patch does not address a similar issue under Windows.
    The reason for this is that the negative margins specified in winstripe led me to believe that the non-native look of the menu was intentional.  Were that not the case, I probably would have specified the padding and margin in browser/base/content as well.

Comment 3

[Bill Gianopoulos [:WG9s]]

Attachment: Screenshots

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 493131 [details] [diff] [review]
patch v1

>--- a/browser/base/content/browser.css
>+++ b/browser/base/content/browser.css
>@@ -188,16 +188,20 @@ toolbar[mode="icons"] > #reload-button[d
> #appmenu-toolbar-button > .toolbarbutton-text {
>   display: -moz-box;
> }
> %endif
> 
> #appmenu_offlineModeRecovery:not([checked=true]) {
>   display: none;
> }
>+
>+#appmenuPrimaryPane {
>+  -moz-border-end: 1px solid ThreeDShadow;
>+}

This doesn't belong in content/.

Comment 5

[Bill Gianopoulos [:WG9s]]

Attachment: patch v2-address review comments

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 494271 [details] [diff] [review]
patch v2-address review comments

The space between menu items and the popup border appears to be OS theme specific. For instance, I'm using the "dust sand" theme where menu items are supposed to touch the border.

Comment 7

[Bill Gianopoulos [:WG9s]]

(In reply to comment #6)
> Comment on attachment 494271 [details] [diff] [review]
> patch v2-address review comments
> 
> The space between menu items and the popup border appears to be OS theme
> specific. For instance, I'm using the "dust sand" theme where menu items are
> supposed to touch the border.

OK I managed to get dust sand installed under fedora and I see what you mean.  Is the menu popup also not supposed to overlap the border so that a double border shows (that is what I am seeing, but am not sure that is how it is supposed to be or if that is an artifact in trying to get an ubuntu theme working under fedora)?

If the popup is not supposed to overlap, I don't think there is much that can be done here in css at least.  If the overlap is supposed to be there, then I guess the best we can do in css is leave the highlight touching the center border and creating the overlap.  If that is what is desired here, I can do a new patch to just accomplish that.

Comment 8

[Marco Castelluccio [:marco]]

Any progress?

Comment 9

[Dão Gottwald [:dao]]

This UI is gone.