613880 - Submenus should overlap a bit

Status: RESOLVED INCOMPLETE

(Toolkit :: Themes, defect)

Description

[Michael Monreal [:micmon]]

Attachment: Screenshot

In GTK, submenus always overlap the parent menu one or two pixels. This way, displaying two borders next to each other (right border of parent next to left border of child) is prevented.

I never really noticed this but in the new firefox button menu, it is very visible now.

Comment 1

[Bill Gianopoulos [:WG9s]]

Attachment: patch v1

Comment 2

[Bill Gianopoulos [:WG9s]]

I don't think this is a widget bug.  It works fine in the right-hand panel and on the traditional menus.  This is just a theme issue in the way the 2 column menu panel is defined.

Comment 3

[Michael Monreal [:micmon]]

(In reply to comment #2)
> It works fine in the right-hand panel and
> on the traditional menus.

It does not, at least for me. But I am nearly sure that this used to work fine, so maybe a regression....

Comment 4

[Bill Gianopoulos [:WG9s]]

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

Cancelling review request as I think I have a better patch coming.

Comment 5

[Bill Gianopoulos [:WG9s]]

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

The other patch didn't work.

Comment 6

[Bill Gianopoulos [:WG9s]]

(In reply to comment #3)
> (In reply to comment #2)
> > It works fine in the right-hand panel and
> > on the traditional menus.
> 
> It does not, at least for me. But I am nearly sure that this used to work fine,
> so maybe a regression....

I wonder if there are 2 issues here.  The offset does not work at all on the left panel of the menus.  That is what I fixed with the theme patch.

I see a one pixel offset in other cases.  I wonder if the widget code does not work for wider than 1 pixel borders.  I will attach screenshots.

Comment 7

[Bill Gianopoulos [:WG9s]]

Attachment: screenshot - right pane showing overlap

Comment 8

[Bill Gianopoulos [:WG9s]]

Attachment: screenshot - traditional menu showing overlap

Comment 9

[Bill Gianopoulos [:WG9s]]

Attachment: screenshot - left pane before patch - no overlap

Comment 10

[Bill Gianopoulos [:WG9s]]

Attachment: left pane - with patch - showing overlap

Comment 11

[Bill Gianopoulos [:WG9s]]

(In reply to comment #3)

> It does not, at least for me. But I am nearly sure that this used to work fine,
> so maybe a regression....

OK so I attached the screenshots showing that I get overlap in all cases except the Firefox menu left panel and that the patch I attached here fixes that issue.

I am not sure why you see something different.

So, the patch here fixes the issue I introduced with the patch in bug 585370.

Sorry I hijacked your bug.  This is what I thought you were complaining about.

Comment 12

[Michael Monreal [:micmon]]

Attachment: Traditional menu - Clearlooks

for completeness I have attached another shot of the traditional menu with the clearlooks theme (current gnome 2.x and fedora default)

Comment 13

[Bill Gianopoulos [:WG9s]]

(In reply to comment #12)
> Created attachment 492218 [details]
> Traditional menu - Clearlooks
> 
> for completeness I have attached another shot of the traditional menu with the
> clearlooks theme (current gnome 2.x and fedora default)

That is very odd.  My screenshots are all with the latest Firefox nightly with default theme and clearlooks OS theme fedora 14 gnome desktop.

Comment 14

[Bill Gianopoulos [:WG9s]]

(In reply to comment #12)
> Created attachment 492218 [details]
> Traditional menu - Clearlooks
> 
> for completeness I have attached another shot of the traditional menu with the
> clearlooks theme (current gnome 2.x and fedora default)

Probably a dumb question, but are you sure you don't have some stray code in userChrome.css adding this extra spacing that is a leftover form debugging some other issue?   (I've been known to do that)

Comment 15

[Bill Gianopoulos [:WG9s]]

Attachment: patch v2

With the previous patch, the highlights of menu items on the left pane had a 1px space on either side, but on the right hand panel, there was only a 1px space on the right.

Comment 16

[Dão Gottwald [:dao]]

Comment on attachment 492230 [details] [diff] [review]
patch v2

As I understand it, this affects all menus, not just those in the app button menu. And even for that menu, this patch doesn't seem to implement what attachment 492208 [details] suggests.

Comment 17

[Bill Gianopoulos [:WG9s]]

(In reply to comment #16)
> Comment on attachment 492230 [details] [diff] [review]
> patch v2
> 
> As I understand it, this affects all menus, not just those in the app button
> menu. And even for that menu, this patch doesn't seem to implement what
> attachment 492208 [details] suggests.

OK.  What happened here is that while testing the pending patch on bug 589146, I noticed an issue in that the sub-menus did not overlap the border between the 2 panes of the Firefox menu.  I thought that was what the complaint here was about.  Obviously I was mistaken.  I will file a new bug on the issue I see, try to explain it better and move the patch to there.

Comment 18

[Michael Monreal [:micmon]]

(In reply to comment #14)
> Probably a dumb question, but are you sure you don't have some stray code in
> userChrome.css adding this extra spacing that is a leftover form debugging some
> other issue?   (I've been known to do that)

Double checked just to make sure but no, this is the way it looks without any modifications.

Comment 19

[Bill Gianopoulos [:WG9s]]

(In reply to comment #18)
> (In reply to comment #14)
> > Probably a dumb question, but are you sure you don't have some stray code in
> > userChrome.css adding this extra spacing that is a leftover form debugging some
> > other issue?   (I've been known to do that)
> 
> Double checked just to make sure but no, this is the way it looks without any
> modifications.

Well then this is baffling.  I have no idea why with such seemingly similar configurations we would be having such diverse results.  I am sure once we figure it out it will be an "Oh Yeah" type moment, but for now, I just don't get it.

Comment 20

[Bill Gianopoulos [:WG9s]]

(In reply to comment #17)
> (In reply to comment #16)
> > Comment on attachment 492230 [details] [diff] [review] [details]
> > patch v2
> > 
> > As I understand it, this affects all menus, not just those in the app button
> > menu. And even for that menu, this patch doesn't seem to implement what
> > attachment 492208 [details] [details] suggests.
> 
> OK.  What happened here is that while testing the pending patch on bug 589146,
> I noticed an issue in that the sub-menus did not overlap the border between the
> 2 panes of the Firefox menu.  I thought that was what the complaint here was
> about.  Obviously I was mistaken.  I will file a new bug on the issue I see,
> try to explain it better and move the patch to there.

Actually, based on further testing under Windows, I have determined that the entire margin/border highlighting issue is platform independent. Therefore it will probably take longer than I thought to come up with new patches.  I will file one to make Windows be consistent in the way it highlights between the traditional Firefox menu and the new menu, and once that I get hat sorted, I will file a bug on gnomestripe to land over that code to fix any remaining Linux issues.

Comment 21

[Michael Monreal [:micmon]]

No longer relevant with Photon theme.