420381 - Vista content area dropdowns often don't show the dropdown arrow

Status: RESOLVED WONTFIX

(Core :: Widget: Win32, defect, P2)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Content area dropdowns aren't invalidating correctly for some reason, and it looks like my code in GetWidgetOverflow here: http://mxr.mozilla.org/seamonkey/source/widget/src/windows/nsNativeThemeWin.cpp#1577 is to blame.  Commenting out that chunk and everything works correctly (except, I presume, if we need to invalidate the 1px that we do draw overflow for).

The problem is that aFrame->GetSize() is returning 0x0.  roc, any idea why that is?  From reading the call site at http://mxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#5291 it seems like the theme is queried for the overflow before the frame's size is determined.. so we end up with 0x0.  Is there any way to do this correctly?  I just need to inform layout that this widget wants to paint a few pixels beyond its normal size border.

Making this a P1 blocker for b4 -- easy b4 fix would be to just comment out the chunks in GetWidgetOverflow, since we'd get 99% of the benefits here and only a mild visual issue, but this shouldn't be hard to fix correctly.

Comment 1

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

Best fix here would be to pass the new size in as a parameter to GetWidgetOverflow. Cocoa seems to have a bug here too, not sure why it didn't show up there. FinishAndStoreOverflow has the new size in aNewSize.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

(marking tm=beta 4 as per comment 0)

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ok, I pass aNewSize down, but it looks wrong at this point -- the size I'm getting is 17x2 for DROPDOWN_BUTTON, which is wrong -- my expansion turns this into 18x4, and the 4 is too small.  When we get to DrawWidgetBackground, the size that's passed in is always 18x20, which is correct.

Any idea why GetWidgetOverflow just doesn't return nsMargin that gets added to its size to compute the overflow?  All three current users always just compute nsMargin and then inflate the frame size by it.  I'll look into this some more.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: let's bandaid for b4

Let's do this bandaid for b4 -- just disable the overflow computation for this widget.  The potential for visual artifacts is extremely small; I couldn't figure out how to even construct it, since it would require the 1px border to be invalidated while the widget is being hovered.  This might even be ok to ship for final if I can't figure out what the issue with with GetWidgetOverflow.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This problem actually has some more effects -- I noticed that if a dropdown in content is open and the mouse is moved in/out of the dropdown menu, the dropdown arrow highlights/unhighlights as if it was being hovered itself.  This is because in the native theme code, dropdown arrow state comes from the parent frame, which is wrong for this case.  However, checking for the hover state on the dropdown arrow frame itself caused it to never become hovered!  I think there's something wacky going on here.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 306916 [details] [diff] [review]
let's bandaid for b4

This has a=beltzner for 1.9b4 once reviewed. roc, if you can get to it in time for nightlies, that'd be great.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Bandaid checked in, retargeting further work for final.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I'm going to push this out -- I spent another hour or so poking at it, but I can't get what's currently checked in to break in any way.  So let's not worry about this for 1.9 final.