Opened 6 years ago

Closed 6 years ago

#17344 closed defect (fixed)

regression - anchor tags are broken in menus

Reported by: Randy Hudson Owned by: bill
Priority: undecided Milestone: 1.10
Component: Dijit Version: 1.8.4
Keywords: Cc:
Blocked By: Blocking:

Description

We use anchor tags in our navigation menubar's menuitems to give the user the option of keeping the current page open and navigating in a new browser tab/window. This can be done by holding down CTRL/COMMAND, or using the middle mouse button.

After upgrading (from 1.6.x), both mechanisms seem to be broken in most browsers. Here is an example snippet you can insert into a parsed HTML page (like the MenuBar? example):

	<div data-dojo-type="dijit.MenuBar" id="navMenu">
		<div data-dojo-type="dijit.PopupMenuBarItem" data-dojo-props="label:'File'">
			<div data-dojo-type="dijit.Menu" id="fileMenu">
				<div data-dojo-type="dijit.MenuItem" onClick="alert('file 1')">
					Click this: <a href="https://bugs.dojotoolkit.org/">dojotoolkit</a>
				</div>
				<div data-dojo-type="dijit.MenuItem" onClick="alert('file 2')">
					File #2
				</div>
			</div>
		</div><div data-dojo-type="dijit.PopupMenuBarItem">
			<span>
				Edit
			</span>
			<div data-dojo-type="dijit.Menu" id="editMenu">
				<div data-dojo-type="dijit.MenuItem" onClick="alert('edit 1')">
					Edit #1
				</div>
				<div data-dojo-type="dijit.MenuItem" onClick="alert('edit 2')">
					Edit #2
				</div>
			</div>
		</div>
	</div>

The issue seems to be in _MenuBase's postCreate, the lines:

	on(this.containerNode, on.selector(matches, a11yclick), function(evt){
		self.onItemClick(registry.byNode(this), evt);
		evt.stopPropagation();
		evt.preventDefault();
	})

Change History (6)

comment:1 Changed 6 years ago by bill

Sounds like you are on a mac?

Well, we've never claimed to support anything other than plain text in menu items, but OTOH it sounds like a11yclick is matching more than it should. Need to check whether or not the browser generates vanilla click events when you press the middle button or do a COMMAND-click.

comment:2 Changed 6 years ago by Randy Hudson

When there is nothing but plain text in a Menu, what does calling evt.preventDefault() prevent?

comment:3 Changed 6 years ago by bill

It might be there to prevent the screen jump issue (I think this was on IE, maybe other browsers too), when you scroll down on a page, focus a button, and then type the space or enter key.

Or perhaps it's because that code used to be calling dojo.stopEvent() and I changed it to no longer depend on that deprecated method... in which case the call to preventDefault() is not intentional.

Even if the preventDefault() call weren't there though, it would still be calling self.onItemClick(registry.byNode(this), evt);

comment:4 Changed 6 years ago by bill

I tested Menu on both 1.6 and trunk, in both versions it responds to click events with the middle mouse button, not just the left mouse button. Although this behavior surprised me, it seems proper as it matches the behavior of a plain HTML button.

Also, about the _MenuBase.postCreate() snippet you pasted, the equivalent code in 1.6 was in MenuItem?.js:

_onClick: function(evt){
	// summary:
	//		Internal handler for click events on MenuItem.
	// tags:
	//		private
	this.getParent().onItemClick(this, evt);
	dojo.stopEvent(evt);
},

Note that it's calling stopEvent(), which does both a stopPropagation() and preventDefault(). That's why the preventDefault() is in the current code. I agree the preventDefault() is likely unnecessary, but I don't see how the behavior could have changed since 1.6.

Anyway, I could remove the preventDefault() call in the trunk branch; would that help you?

comment:5 Changed 6 years ago by bill

Milestone: tbd1.10
Owner: set to bill
Status: newassigned

No answer. I'll just remove that preventDefault().

comment:6 Changed 6 years ago by Bill Keese <bill@…>

Resolution: fixed
Status: assignedclosed

In 622f2a8e8d6c425c7fae37af86c73858978c6383/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.