103069 - context menu shifts on dismissal

Status: NEW

(Core :: XUL, defect)

Description

[:deanis74]

This may have already been filed, perhaps even by me, but I can't find it.

Steps to reproduce:

1. Find an old, slow computer like my PII-350
2. Start Composer
3. Right-click to display the context menu over top of Edit Mode Toolbar
4. Note that there are four menu items
5. Press Esc
6. Watch closely

Expected Results: menu dismisses

Actual Results: menu jumps up a bunch and then dismisses

I only ever see this with the context menu in Composer.

Comment 1

[Jean-Francois Ducarroz]

over to editor

Comment 2

[:deanis74]

This also only happens if I dismiss the menu using Esc.  If I dismiss it with a
left button press, no jumpiness.

Comment 3

[:deanis74]

I wonder if this is related to bug 105698 in some weird way?

Comment 4

[Dan Soper]

I see this bug in the browser - build 2001122106, win98, PII/350MHz

Comment 5

[Dan Soper]

...forgot to mention, I have seen this menu-jump when the manu is dismissed by
*any means* - hit Esc, click somewhere outside the menu, or click on a menu item.

Comment 6

[djk]

I, too, have noticed it in Browser, PII 333.

Happens in case:
1) right click
2) hold button
3) move pointer
4) release button
5) dismiss context menu

Doesn't happen in case:
1) right click
2) do NOT move the mouse between mousedown and mouseup
3) dismiss context menu

Comment 7

[Sergi]

See last comment for bug 105698. Probably, the nature of both bugs are the same.

Comment 8

[Chris Abbey]

*** Bug 144015 has been marked as a duplicate of this bug. ***

Comment 9

[:deanis74]

--> XPMenus based on dupe bug 144015

Comment 10

[Chris Abbey]

bringing over cc: list from my dup

Comment 11

[Peter Trudelle]

nsbeta1- per Nav triage team

Comment 12

[Thomas Schoepf]

Here's what I get:

XXX Damage rectangle (38,4237,3477,19) does not intersect the widget's view
(0,0,3477,4218)!
###!!! ASSERTION: ACK, BAD TARGET NODE: 'targetNode', file nsMenuPopupFrame.cpp,
line 600
###!!! Break: at file nsMenuPopupFrame.cpp, line 600
###!!! ASSERTION: ACK, BAD TARGET: 'targetDocumentWidget', file
nsMenuPopupFrame.cpp, line 623
###!!! Break: at file nsMenuPopupFrame.cpp, line 623

This is the output of m1rc2 (Linux, self compiled) after:
1. Right click in the browser window
2. Move the mouse cursor down and up again through the popup menu
3. Press ESC

Comment 13

[djk]

I don't see this anymore on build 2002051908-trunk, win98!

Fixed somehow?

Comment 14

[Chris Abbey]

The code that threw the assert in question two posts above was changed the other
day under bug 143607, one of the effects of that bug was the duplicate menu
issue this bug was all about... propose therefore we dup this (and possibly
105698) to 143607 and call it closed.

people to thank for the fix: p=bryner r=hewitt sr=jst a=shaver, none of whom are
on either of these bugs. :)

Comment 15

[Mike Kowalski]

This bug is fixed.

Comment 16

[:deanis74]

No it's not.  I can still reproduce using the original steps with build
2002060308 on Win2K.

Comment 17

[Mike Kowalski]

Yeah, you're right. I think it's fixed in navigator though.

Comment 18

[Kyle Yuan]

I got it!

Comment 19

[Kyle Yuan]

Attachment: remove EditorCleanupContextMenu

Comment 20

[Kyle Yuan]

cmanske:

Why do we need EditorCleanupContextMenu? It causes the hidden menuitems to get 
appeared again. In fact, they are not shown on screen, but are calculated when 
the popup menu is reflowing. So the popup menu becomes larger and wider, and 
jumps if its new frame can't be located completely within the screen.

Are there any disadvantages if we leave hidden and contexthidden attribute here?

Comment 21

[Kyle Yuan]

Attachment: better fix - commented EditorCleanupContextMenu

Comment 22

[Charles Manske]

I've always wondered what purpose this served! I vote for ripping out
EditorCleanupContextMenu() and ShowHiddenItemOnCleanup() entirely!
When we create a context menu, we always recalculate the enable/disabled states
and show/hide items appropriately. I don't think we need any "cleanup."

Comment 23

[Charles Manske]

Attachment: Suggestion to remove all cleanup code

Comment 24

[Kyle Yuan]

Comment on attachment 92306 [details] [diff] [review]
Suggestion to remove all cleanup code

r=kyle
cmanske's change is ok to me.

Comment 25

[Kyle Yuan]

=> cmanske

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.