844255 - menupopup is transparent, menuitem are invisible in certain case.

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Alice0775 White]

Attachment: testcase

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/885cde564ff3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130222 Firefox/22.0 ID:20130222031133 

Steps to Reproduce:
1. Enable Windows7 Aero visual style
2. All checked "Performance options" tabsheet "Visual effects" from Control Panel

3. Start Firefox with clean profile
4. Disable HWA from option dialog and restart browser

5. Open testcase

6. Mouse hover "toolbar" and Click "Menu1" to open menupopup
   --- observe popup menuitem are visible
7. Move mouse to contentarea so that the menupopup auto hide
   (but shadow remains --- another bug at least since 4.0b2pre)
8. Click empty area of contentarea
9. Mouse hover toolbar and Click "Menu1" again
   --- observe menupopup and menuitem

Actual Results:
menupopup and menuitem are invisible.

Expected  Results:
menupopup menuitem should be visible.



Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/e6df45a28902
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121007183233
Bad:
http://hg.mozilla.org/mozilla-central/rev/24cf40690042
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121008010834
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e6df45a28902&tochange=24cf40690042


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/65b95ed309b9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121007175634
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/7c119b50e7aa
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121007183935
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=65b95ed309b9&tochange=7c119b50e7aa


Last Good: 65b95ed309b9\
First Bad: 2d39dbbe75b3\
Triggered by:
	2d39dbbe75b3	James H — Bug 610713 - Fix popup menus leaving behind artifacts when using hardware acceleration and a basic Windows theme. r=jmathies


Try build of Bug 823028 Comment#17 fixes the problem
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-7ef75d6483da

Comment 1

[Alice0775 White]

Attachment: screenshot

Comment 2

[James]

Attachment: patch from try build

This is the patch from that try build.

I *think* all the related bugs only occurred when HWA was on and Aero was off, so only compositing the popups when accel is on should be safe.

nsWindow::HasBogusPopupsDropShadowOnMultiMonitor() was the only way I could find to detect if accelerated layers were actually being used because it checks the prefs. (In my case hwa is blocked unless I force enable the prefs)

I also toyed around with disabling the flag when Aero was enabled, but the flag would need to be toggled in nsWindow::Show in response to WM_COMPOSITIONCHANGED. I could pursue that if you want.

Comment 3

[Jim Mathies [:jimm]]

Comment on attachment 717511 [details] [diff] [review]
patch from try build

I'm not familiar with HasBogusPopupsDropShadowOnMultiMonitor. Maybe Bas has an opinion on this.

Comment 4

[Jim Mathies [:jimm]]

This feels more like a layout bug. It looks as if the popup widget is still present, but we're not rendering anything to it. So all you're left with is a drop shadow.

Comment 5

[Alice0775 White]

FYI,
When I add the following CSS to userChrome.css, this problem is revised

menupopup { visibility: visible !important; }

Comment 6

[James]

Attachment: patch

Unbitrotting and switching to review request. This essentially backs out bug 610713 when layers acceleration is disabled which seems to be causing invisible popups for some users (Also see bug 872466 and bug 823028). Try builds based on this patch in those bugs and this one seem to resolve it.

It's been awhile since I looked at this, but if I recall HasBogusPopupsDropShadowOnMultiMonitor() was the only way to check if layers accel was actually being used in cases where it may have been blocked even though HWA was enabled. The original feedback request was mostly regarding the function name and if using it was an acceptable way to make sure layers accel wasn't being used.

Comment 7

[Alice0775 White]

almost 1 month...
no progress here...

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 753320 [details] [diff] [review]
patch

Review of attachment 753320 [details] [diff] [review]:
-----------------------------------------------------------------

No idea why I missed this review request. Sorry!

Comment 9

[Alice0775 White]

Please checkin to m-c before next merge day.
I think that the last chance of the fix for next 24.0esr.

Comment 10

[James]

Patch still applies cleanly to latest m-c.

I don't have bug editing permissions, could you switch the checkin-needed flag to a keyword? Not sure if committers search the whiteboard field

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to James from comment #10)
> I don't have bug editing permissions,

Fixed.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/414ba17d94d1

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/414ba17d94d1

Comment 14

[Alice0775 White]

I cannot reproduce in Nightly25.0a1 and Aurora24.0a2 anymore.

http://hg.mozilla.org/mozilla-central/rev/cc80aa0c7c13
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130626 Firefox/25.0 ID:20130626031100
http://hg.mozilla.org/releases/mozilla-aurora/rev/17666746e8cc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130626 Firefox/24.0 ID:20130626004017

Comment 15

[Alice0775 White]

Can the patch up-lift to beta23.0 channel?

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 753320 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 610713
User impact if declined: popup menus broken for some users
Testing completed (on m-c, etc.): just landed
Risk to taking this patch (and alternatives if risky): reasonably low risk, this is basically a partial backout of 610713 for users who don't need it
String or IDL/UUID changes made by this patch: none

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f6170a2bdfb7

Comment 18

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0