579264 - Selected menu item not repainted when the menu is closed and then reopened

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

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

I sometimes see this in retained-layers builds. I strongly suspect that we simply aren't invalidating menu items sometimes when they're activated and the menu closes; this wouldn't cause any problems pre-retained-layers, because we'd always redraw the menu again next time, but with retained layers next time we show the menu we may be drawing it from a retained layer buffer.

Unfortunately I have no reliable steps to reproduce. I'm kinda hoping someone else can come up with some.

Comment 1

[Alice0775 White]

I can confirm on Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre) Gecko/20100716 Minefield/4.0b2pre ID:20100716040830

[STR]
1. Start Minefield with new profile
2. Make folder under Bookmarks button
3. Make enough number of bookmarks in the folder

4. Open Bookmarks button
5. Open the folder
6. Move mouse into folder's popup and move to downward and hover over bookmark (a bookmark is highlighted)
7. Move mouse to Bookmarks button's popup(then folder's popup will close)
8. Open folder again and Move mouse into folder popup

[Actual]
The last highlight is left

[Expected]
The last highlight  should be removed

Comment 2

[Alice0775 White]

Attachment: screen shot ogg movie

Comment 3

[Alice0775 White]

This problem happens in not only bookmarks menu but also View menu in the menubar etc.

And it happens on Lunux too. --- All
Mozilla/5.0 (X11; Linux i686; en-US; rv:2.0b2pre) Gecko/20100716 Firefox/4.0b2pre ID:20100716025911

Comment 4

[rsx11m]

I can also reproduce in the Mail & News window of Mozilla/5.0 (Windows; Windows NT 5.1; en-US; rv:2.0b2pre) Gecko/20100717 SeaMonkey/2.1a3pre, in addition to the menus of the browser window. In general, it seems to be more frequently the case when I first click on the menu to make it "stick", then go down with the mouse to select a menu item with a second click. Reopening the same menu after that may show that item still highlighted. If I proceed with another menu item *above* the first one (i.e., not moving across the first one) and click it, both items will be highlighted the next time I open that window, etc. I'm using Windows Classic desktop theme, but apparently this wouldn't make a difference.

Comment 5

[rsx11m]

> both items will be highlighted the next time I open that window
s/window/menu/ ...

Comment 6

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

I can't reproduce this on latest-nightly, Windows 7, fwiw.

Comment 7

[Justin Dolske [:Dolske]]

I see this on the latest-nightly, Windows 7. Though I'm RDPing into my Windows box, so there's no Aero. Though it's just the menuitem outline-on-hover, so hovering any menuitem seems to reset it.

1) Click Firefox button
2) Click Help item
3) Click Firebox button again
4) Notice that Help still have the hover effect applied to it.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Roc, is qawanted still valid? If yes, which information can we supply?

Comment 10

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

Sorry, I've got enough data here. Thanks!

Comment 11

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

I think there's an existing bug here with restyling that just got revealed by retained layers. It seems that sometimes restyling of menu items doesn't generate the correct change hints when the _moz_menuactive attribute is removed while the menu is closed. I'm still looking into it.

Comment 12

[Markus Stange [:mstange]]

I've seen something similar happen with the checked attribute: Menuitems that have been shown in the unchecked state once didn't get a checkmark when the checked attribute was set while the popup was closed.
That already happened before retained layers and I've been meaning to file it ever since the Summit...

Comment 13

[Markus Stange [:mstange]]

Attachment: checkmark testcase

Not sure if this helps... after clicking the menuitem it should be checked on the next opening.

Comment 16

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

Attachment: fix

As discussed with Boris on IRC, style code assumes that ApplyRenderingChangeToTree will repaint every frame in the given frame subtree (including following placeholders to their out-of-flows), but popups don't have placeholders and aren't included in ancestor frames' overflow area, so that repainting doesn't happen.

Comment 17

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

I just realized there's another problem as well. Even ignoring out -of-flows, DoApplyRenderingChangeToTree will invalidate ThebesLayers for the root of the frame subtree but there might also be descendants that render into different ThebesLayers that also need to be invalidated.

Comment 18

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

Filed bug 581317 on that issue.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 459753 [details] [diff] [review]
fix

r=me

Comment 21

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

http://hg.mozilla.org/mozilla-central/rev/30239e4cebd8

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (X11; Linux i686; rv:2.0b3pre) Gecko/20100724 Minefield/4.0b3pre

Could this be tested with an automated test?

Comment 23

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

There's no good way to capture the rendered contents of a popup.