Closed Bug 798157 Opened 7 years ago Closed 7 years ago

awesome bar dropdown background color is missing and looks wrong

Categories

(Core :: Widget: Gtk, defect)

x86_64
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla19
Tracking Status
firefox18 --- verified

People

(Reporter: tnikkel, Assigned: karlt)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(5 files)

I can't provide a screenshot easily because the dropdown disappears when I hit print screen.

Confirmed to be caused by one of the changesets in bug 408284.
You can probably get a delayed screenshot. If you're using GNOME, you do this by running something like "gnome-screenshot -d 5", then open the dropdown and wait for the delay to expire.
Attached image screenshot
Thanks for the tip. Here is a screenshot.
The first bad revision is:
changeset:   109106:4aac63aa19dc
user:        Chris Coulson <chris.coulson@canonical.com>
date:        Wed Oct 03 19:53:53 2012 +1300
summary:     b=408284 use ARGB visuals for popup windows when window manager is compositing r=karlt
I'm not too sure what the background colour is meant to be for your theme, but I think the issue is that there is no shadow. I noticed this earlier too, but it's less obvious in my theme.

Is this using compiz btw? I'd actually just pinged our compiz maintainer to ask if it's possible to make it draw a shadow for argb windows, but I'm still waiting for him to answer.
This is what is should look like.

Yes, I think it is with compiz.
Thanks. This is on Ubuntu, isn't it? Do you know which version it is?
Yes, it is Ubuntu 11.04.
Sounds similar to bug 781332.
Guessing 4aac63aa19dc is just bringing ARGB formats/visuals into the equation, as would have already been there for shaped panels such as the downloads panel.
(Doesn't explain why the issue wasn't seen on identity or bookmarks panels.)
Depends on: 781332
It does seem similar to bug 781332 but it does feel and look different from bug 781332 though.
The text looks awful in the awesome bar dropdown. The text looked fine in the downloads panel.
So I guess there's actually more than one issue here (the lack of shadows for opaque, rectangular windows with an argb visual being one of them) :(
I've now seen the same problem on a select drop down on a content web page, which should make this easier to debug.
Attached file testcase
I wonder what theme is used here?
Perhaps that is in ~/.gtkrc-2.0, or /etc/gtk-2.0/gtkrc.
In my Appearance Preferences the theme is called Human.
(In reply to Karl Tomlinson (:karlt) from comment #14)
> I wonder what theme is used here?
> Perhaps that is in ~/.gtkrc-2.0, or /etc/gtk-2.0/gtkrc.

Neither file exists here.
From a NSPR_LOG_MODULES=Layers:9 log
-3442880[7f0efea62260]: BasicLayerManager (0x7f0ecd95f200)
-3442880[7f0efea62260]:   BasicContainerLayer (0x7f0ed56b1000) [visible=< (x=0, y=0, w=444, h=275); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=444.000000, h=275.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=0.000000, h=0.000000) scrollId=1 }]
-3442880[7f0efea62260]:     BasicThebesLayer (0x7f0ed737ec00) [visible=< (x=0, y=0, w=444, h=275); >] [opaqueContent] [isFixedPosition]

So we think the content is opaque at least.
And mIsTransparent is false and mTransparencyBitmap is null on the window for the problem popup.
I tried this with the same theme today, and wasn't able to reproduce it :(

Does it happen if you set a background color with an inline style on the PopupAutoCompleteRichResult panel (in browser.xul), rather than using the native styled background? Eg, something like "-moz-appearance:none;background-color:lightgray;"
For the other issue (missing shadows), I wonder if we could turn off argb visuals for popups that don't need it? It looks like that information is available before we create the widget for popup windows:

http://hg.mozilla.org/mozilla-central/file/acd25563db2f/layout/xul/base/src/nsMenuPopupFrame.cpp#l265
(In reply to Chris Coulson from comment #19)
> I tried this with the same theme today, and wasn't able to reproduce it :(

Thanks for trying.

> Does it happen if you set a background color with an inline style on the
> PopupAutoCompleteRichResult panel (in browser.xul), rather than using the
> native styled background? Eg, something like
> "-moz-appearance:none;background-color:lightgray;"

That seemed too hard so I just made nsIFrame::IsThemed always return false and I also added the rule  * {-moz-appearance:none !important;background-color:lightgray !important; } to the testcase here and I was still able to reproduce the problem in the testcase.
(In reply to Chris Coulson from comment #20)
> For the other issue (missing shadows), I wonder if we could turn off argb
> visuals for popups that don't need it?

If mPopupHint is useful, it could be used from nsWindow::Create.
If not, then adding another field to nsWidgetInitData sounds appropriate.
(In reply to Karl Tomlinson (:karlt) from comment #22)
> (In reply to Chris Coulson from comment #20)
> > For the other issue (missing shadows), I wonder if we could turn off argb
> > visuals for popups that don't need it?
> 
> If mPopupHint is useful, it could be used from nsWindow::Create.
> If not, then adding another field to nsWidgetInitData sounds appropriate.

Hi,

The problem with mPopupHint is that the awesomebar dropdown is implemented as a panel. I did try a quick test last night using mDropShadow (which gets set on popups without transparent content). That seemed to work ok - the only exceptions being that popups for HTML select elements still get an argb visual when it's not needed, and tooltips no longer get one. Although, tooltips don't really need it yet anyway.
(In reply to Timothy Nikkel (:tn) from comment #21)
> (In reply to Chris Coulson from comment #19)
> > I tried this with the same theme today, and wasn't able to reproduce it :(
> 
> Thanks for trying.
> 
> > Does it happen if you set a background color with an inline style on the
> > PopupAutoCompleteRichResult panel (in browser.xul), rather than using the
> > native styled background? Eg, something like
> > "-moz-appearance:none;background-color:lightgray;"
> 
> That seemed too hard so I just made nsIFrame::IsThemed always return false
> and I also added the rule  * {-moz-appearance:none
> !important;background-color:lightgray !important; } to the testcase here and
> I was still able to reproduce the problem in the testcase.

I don't know if that changes the background style of the popup. I wanted to see if the problem went away with gfxXlibNativeRenderer eliminated from the render path
(In reply to Chris Coulson from comment #24)
> > That seemed too hard so I just made nsIFrame::IsThemed always return false
> > and I also added the rule  * {-moz-appearance:none
> > !important;background-color:lightgray !important; } to the testcase here and
> > I was still able to reproduce the problem in the testcase.
> 
> I don't know if that changes the background style of the popup. I wanted to
> see if the problem went away with gfxXlibNativeRenderer eliminated from the
> render path

I think it should. nsIFrame::IsThemed is what we use to decide if we should use native theme drawing based on a -moz-appearance style rule.
(In reply to Timothy Nikkel (:tn) from comment #25)
> > I don't know if that changes the background style of the popup. I wanted to
> > see if the problem went away with gfxXlibNativeRenderer eliminated from the
> > render path
> 
> I think it should. nsIFrame::IsThemed is what we use to decide if we should
> use native theme drawing based on a -moz-appearance style rule.

There were some cases that bypassed IsThemed and just used ThemeSupportsWidget. After I fixed that there were no calls to any NativeRenderer Draw* function and the problem still happens.
(In reply to Chris Coulson from comment #23)
> I did try a quick test last night using mDropShadow (which gets
> set on popups without transparent content). That seemed to work ok - the
> only exceptions being that popups for HTML select elements still get an argb
> visual when it's not needed, and tooltips no longer get one. Although,
> tooltips don't really need it yet anyway.

mDropShadow sounds a pretty close match for what we want, because we are choosing the XRGB visual to get a drop shadow.

I didn't expect select elements to have drop shadows because I think of them as extending from the button, and the compositor doesn't know whether these extend up or down from the button.  (Perhaps others might imagine or different themes present these differently, though.)

I wonder why tooltips no longer get one.
Is that because mIsContentShell is set or because nsLayoutUtils::GetFrameTransparency doesn't think the frame is styled with transparent background?
I wonder why the mIsContentShell test is even there.  Perhaps GetFrameTransparency cannot be relied on to make content-controlled backgrounds opaque.  (I don't know which popups can be styled by content.)
Tooltips currently don't get one because of http://hg.mozilla.org/mozilla-central/file/05e1bb4d7cd8/widget/gtk2/nsNativeThemeGTK.cpp#l1428.

I'm not sure what to do with the background colour issue at this point. I wish I could reproduce it here. I assume this is happening with the same setup as in bug 781332 (ie, with the fglrx driver)?
(In reply to Chris Coulson from comment #28)
> I'm not sure what to do with the background colour issue at this point. I
> wish I could reproduce it here. I assume this is happening with the same
> setup as in bug 781332 (ie, with the fglrx driver)?

Yes, that is correct, same machine.
Attached file gfx info dump
I looked into this with tn.  When _cairo_xlib_surface_composite is copying a depth-24 opaque pixmap to the depth-32 window, the pixmap looks as expected.
The bug occurs with CAIRO_OPERATOR_SOURCE or CAIRO_OPERATOR_OVER.  There is no mask.

This looks like a bug in the RENDER driver implementation not filling in the alpha components of the destination surface.  The bug did not occur with GL layers.
The bug occurs with compiz or metacity (with compositing).
Blocks: 693168
Using an XRGB visual where possible should be more efficient and allows the
window compositor to know where to draw a drop shadow.

This should also work around the fglrx bug in at least most cases.
This bug at least isn't common on dropdowns that we expect to be transparent,
probably because we use an ARGB Pixmap and so don't usually hit the same
driver bug.  Perhaps there may still be a bug when copying from an XRGB Pixmap
to an ARGB Pixmap (bug 781332) but that shows much less frequently.

Reusing mDropShadow for this wouldn't workaround the fglrx bug for menulist
popups.

I removed the comment about the composited-changed signal because it is not
really correct: even windows that are only shown for a short time tend to be
kept after hidden and reshown at a much later time.  If the window manager is
no longer compositing at a later time, shapes will be added in OnExposeEvent.
Assignee: nobody → karlt
Status: NEW → ASSIGNED
Attachment #681297 - Flags: review?(roc)
Blocks: 807176
https://hg.mozilla.org/mozilla-central/rev/3f605e71a350
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Duplicate of this bug: 807080
Comment on attachment 681297 [details] [diff] [review]
use ARGB visuals only for popups that will be translucent

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 408284
User impact if declined: missing shadows on menus/dropdowns for Ubuntu users + translucent menus for those using fglrx driver
Testing completed (on m-c, etc.): on m-c 
Risk to taking this patch (and alternatives if risky): low - this reduces the number of situations where the changes in bug 408284 take effect
String or UUID changes made by this patch: none.
Attachment #681297 - Flags: approval-mozilla-aurora?
Comment on attachment 681297 [details] [diff] [review]
use ARGB visuals only for popups that will be translucent

low risk patch fixing 807176 tracking 18. Approving on aurora.
Attachment #681297 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Adding QA to help verify issue in this bug is fixed and as well and a quick verification on Bug 807176 being fixed by the patch uplifted here.
Keywords: qawanted, verifyme
QA Contact: jbecerra
Duplicate of this bug: 807176
Couldn't reproduce the issue from comment 0 with fglrx.

Could anyone who could reproduce set this to verified?
This is fixed for me. Yay.
Status: RESOLVED → VERIFIED
Keywords: qawanted, verifyme
You need to log in before you can comment on or make changes to this bug.