Closed
Bug 798157
Opened 12 years ago
Closed 12 years ago
awesome bar dropdown background color is missing and looks wrong
Categories
(Core :: Widget: Gtk, defect)
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)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
roc
:
review+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Comment 1•12 years ago
|
||
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.
Reporter | ||
Comment 2•12 years ago
|
||
Thanks for the tip. Here is a screenshot.
Reporter | ||
Comment 3•12 years ago
|
||
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
Comment 4•12 years ago
|
||
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.
Reporter | ||
Comment 5•12 years ago
|
||
This is what is should look like.
Yes, I think it is with compiz.
Comment 6•12 years ago
|
||
Thanks. This is on Ubuntu, isn't it? Do you know which version it is?
Reporter | ||
Comment 7•12 years ago
|
||
Yes, it is Ubuntu 11.04.
Assignee | ||
Comment 8•12 years ago
|
||
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
Reporter | ||
Comment 9•12 years ago
|
||
It does seem similar to bug 781332 but it does feel and look different from bug 781332 though.
Reporter | ||
Comment 10•12 years ago
|
||
The text looks awful in the awesome bar dropdown. The text looked fine in the downloads panel.
Comment 11•12 years ago
|
||
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) :(
Reporter | ||
Comment 12•12 years ago
|
||
I've now seen the same problem on a select drop down on a content web page, which should make this easier to debug.
Reporter | ||
Comment 13•12 years ago
|
||
Assignee | ||
Comment 14•12 years ago
|
||
I wonder what theme is used here?
Perhaps that is in ~/.gtkrc-2.0, or /etc/gtk-2.0/gtkrc.
Reporter | ||
Comment 15•12 years ago
|
||
In my Appearance Preferences the theme is called Human.
Reporter | ||
Comment 16•12 years ago
|
||
(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.
Reporter | ||
Comment 17•12 years ago
|
||
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.
Reporter | ||
Comment 18•12 years ago
|
||
And mIsTransparent is false and mTransparencyBitmap is null on the window for the problem popup.
Comment 19•12 years ago
|
||
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;"
Comment 20•12 years ago
|
||
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
Reporter | ||
Comment 21•12 years ago
|
||
(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.
Assignee | ||
Comment 22•12 years ago
|
||
(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.
Comment 23•12 years ago
|
||
(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.
Comment 24•12 years ago
|
||
(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
Reporter | ||
Comment 25•12 years ago
|
||
(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.
Reporter | ||
Comment 26•12 years ago
|
||
(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.
Assignee | ||
Comment 27•12 years ago
|
||
(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.)
Comment 28•12 years ago
|
||
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)?
Reporter | ||
Comment 29•12 years ago
|
||
(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.
Reporter | ||
Comment 30•12 years ago
|
||
Assignee | ||
Comment 31•12 years ago
|
||
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.
Assignee | ||
Comment 32•12 years ago
|
||
The bug occurs with compiz or metacity (with compositing).
Assignee | ||
Comment 33•12 years ago
|
||
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.
Attachment #681297 -
Flags: review?(roc) → review+
Assignee | ||
Comment 34•12 years ago
|
||
Flags: in-testsuite-
Comment 35•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Assignee | ||
Comment 37•12 years ago
|
||
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 38•12 years ago
|
||
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+
Comment 39•12 years ago
|
||
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.
Assignee | ||
Comment 40•12 years ago
|
||
status-firefox18:
--- → fixed
Comment 42•12 years ago
|
||
Couldn't reproduce the issue from comment 0 with fglrx.
Could anyone who could reproduce set this to verified?
Reporter | ||
Comment 43•12 years ago
|
||
This is fixed for me. Yay.
Updated•12 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•