Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#1076 closed defect (fixed)

Popupmenu broken in FF and konq see test_Menu2.html

Reported by: mumme Owned by: anonymous
Priority: high Milestone:
Component: Widgets Version: 0.3
Keywords: popupmenu, contextmenu Cc:
Blocked By: Blocking:

Description

In konq (3.5.3) it expands in and expands out before any chance to see the menu

In FF (1.5.3 linux) it opens the menu but doesnt prevent the browser contextmenu.

changing from
 evt.preventDefault() 
to 
 dojo.event.browser.stopevent(evt)

Appears to fix it, but there seems to be some code that should handle the onclick that follows the oncontextmenu without stoping the event, so Im hesitant to check it in.

Somebody who knows the code more should take a look.

/ Fredrik

Change History (6)

comment:2 Changed 13 years ago by liucougar

Resolution: fixed
Status: newclosed

confirmed in konq, fixed in r4605

comment:3 Changed 13 years ago by mumme

Resolution: fixed
Status: closedreopened

Well I still see it in my FF 1.5.3 installed from kubuntu dapper package. Se screenshot here: http://dojotoolkit.org/~mumme/screendump.png

Sorry I should have explained myself better, I meant are there any usecases were the page author would want to listen to the event that triggered onOpen or could we safely stop this event.

I was afraid that was the case as there is some code in that *may* be related to this, the array explodeSrc get set to Menu2Manager.currentButton and that is used in the Menu2Manager onClick (which is called somehow)

I didnt mean to stop the onmousedown event in konq_menu_fixer I meant the oncontextmenu event in onOpen of PopUpMenu2.

Index: trunk/src/widget/Menu2.js
===================================================================
--- trunk/src/widget/Menu2.js	(revision 4607)
+++ trunk/src/widget/Menu2.js	(working copy)
@@ -246,9 +246,7 @@
 		}
 		this.open(x, y, null, [x, y]);
 
-		if(e["preventDefault"]){
-			e.preventDefault();
-		}
+		dojo.event.browser.stopEvent(e);
 	}
 });
 
@@ -615,7 +613,7 @@
  				// rightclick, listen to mousedown events
  				node._menufixer_konq = function(e){
  					if (e.button==2 ){
- 						e.stopPropagation(); // need to prevent browsers menu
+ 						e.preventDefault(); // need to prevent browsers menu
  						this.oncontextmenu(e);
  					}
  				};

This works in both browsers, but like I said it may interfere page authors assumptions.

/ Fredrik

comment:4 Changed 13 years ago by bill

Hi Fredrik,

Dojo's convention is to use e.preventDefault() and e.stopPropagation() rather than calling dojo.event.browser.stopEvent() directly. Dojo makes sure that those functions are available in the event object for all browsers.

There may be code in DropDownContainer? or ComboBox? that is listening for right-click events (ie, if you click anywhere on the screen then the current combo box drop down should be closed), but Liucougar is fixing all that code to use Menu2Manager directly, so it shouldn't be an issue. Thus, if you need to call stopPropogation() in addition to calling preventDefault(), I think it's OK.

comment:5 Changed 13 years ago by mumme

Resolution: fixed
Status: reopenedclosed

Ok thank you both, liuCougar and Bill. Sorry for the trouble, I will use e.stopPropagation() in the future.

Just checked in r4616 which uses this.

/ Fredrik

comment:6 Changed 12 years ago by (none)

Milestone: 0.4

Milestone 0.4 deleted

Note: See TracTickets for help on using tickets.