463519 - Setting a bgcolor on #main-window in a theme screws up menus

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Mike Kaply [:mkaply]]

This is a really strange problem that we haven't been able to figure out.

If you set a bgcolor on #main-window in a theme, the separators on menupopups display in a different color (not the bgcolor you set to main-window, just a different color)

This only happens on Mac.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Screen capture

So in looking at the image, notice that the separators are not the same color as the rest of the menu (they stand out)

Comment 2

[Markus Stange [:mstange]]

Do you set the #main-window's -moz-appearance to none?

Comment 3

[Markus Stange [:mstange]]

Oh, you don't even have to do that. I can see the bug.

Comment 4

[Mike Kaply [:mkaply]]

Yeah, moz-appearance has no effect.

Even if I could find something to set in the theme to put the menus back, I would be happy.

Comment 5

[Markus Stange [:mstange]]

I think I've found a workaround. Add this to your theme's popup.css:
menupopup > arrowscrollbox,
popup > arrowscrollbox {
  -moz-appearance: menupopup !important;
}

Explanation: The menupopup -moz-appearance seems to be only ~97% opaque, so the popup's background shines through just a little. Adding another layer of -moz-appearance: menupopup; on top (using the arrowscrollbox) hides the background almost completely.
But I have no idea why the #main-window's background color is painted behind the popup's root frame.

Comment 6

[Stefan [:stefanh]]

Switching to padding instead of margin for the separator seems to work. Try this (not sure if you need the !important rules, I just added them there to be sure):

menuseparator {
   padding: 6px 0 !important;
   margin: 0 !important;
}

Comment 7

[Markus Stange [:mstange]]

Does that also work for the 4px padding at the top and the bottom of the menu?

Comment 8

[Stefan [:stefanh]]

(In reply to comment #7)
> Does that also work for the 4px padding at the top and the bottom of the menu?

Ah, right - no it doesn't.

Comment 9

[Mike Kaply [:mkaply]]

The separator trick doesn't work completely because there are problems at the top and bottom of the menu as well.

Comment 10

[Markus Stange [:mstange]]

Since bug 391984 made the menus transparent, this particular bug won't show anymore. But the underlying issue isn't fixed yet.

Comment 11

[Markus Stange [:mstange]]

The patch in bug 502424 sounds like it might fix the underlying Gecko bug.

Comment 12

[Markus Stange [:mstange]]

Fixed by bug 502424. Thanks Timothy!

Comment 13

[Timothy Nikkel (:tnikkel)]

Markus, what did you use to test this? I'd be interested to know if this was a regression caused by bug 488242 that has regressed before, or if bug 502424 fixed a problem that existed before bug 488242 landed.

Comment 14

[Markus Stange [:mstange]]

This bug existed long before bug 488242 landed. This is what I did to test it:

1. Open a new window, so that the widget for its content context menu hasn't been created yet. (Changing the transparency of a popup window doesn't work after it has been created. We should fix that.)
2. Open the DOM Inspector and inspect this new window.
3. Set style="background-color: red" on the window element.
4. Set style="-moz-appearance: radio" on the element window > #mainPopupSet > #contentAreaContextMenu.
5. Right-click into the window's content area.

Before you fixed bug 502424, the popup's background was red, now it's white.

This is probably a very artificial testcase. Since -moz-appearance: radio has transparency (at least on Mac OS X - no idea if this works on Windows, too), it should make the window transparent, too (like -moz-appearance: menupopup does for context menus). But we return eTransparencyOpaque in nsNativeThemeCocoa::GetWidgetTransparency, so it doesn't.
So you could say that this issue is only exposed by a bug.

Fun fact: Since bug 502424 makes us not draw a background color in these cases any more, the transparent parts aren't cleared when repainting. So you can see the half-transparent pixels get darker over time.

Comment 15

[Markus Stange [:mstange]]

(In reply to comment #14)
> 4. Set style="-moz-appearance: radio"

Before bug 391984, -moz-appearance: menupopup in this place would reproduce the bug, because it's only 97% opaque.

Comment 16

[Timothy Nikkel (:tnikkel)]

(In reply to comment #14)
> Fun fact: Since bug 502424 makes us not draw a background color in these cases
> any more, the transparent parts aren't cleared when repainting. So you can see
> the half-transparent pixels get darker over time.

Can you try something for me? In PresShell::AddCanvasBackgroundColorItem after the bgcolor is declared, add
  if (!nsCSSRendering::IsCanvasFrame(aFrame))
    bgcolor = NS_RGBA(0,0,0,0);
and comment out the earlier IsCanvasFrame if statement, and see if that fixes this darkening issue and doesn't regress anything else.

Comment 17

[Markus Stange [:mstange]]

It doesn't fix it. But I don't think it needs fixing: If GetWidgetTransparency says that the widget is completely opaque we shouldn't waste CPU cycles clearing the background. Instead we should fix GetWidgetTransparency to report correct values.