Opened 10 years ago

Closed 8 years ago

#10250 closed defect (fixed)

Identity check for dijitclick handling breaks in Greasemonkey environment

Reported by: Mark Wubben Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.4.0b
Keywords: Cc:
Blocked By: Blocking:

Description

I'm initializing a Dijit Menu from a Greasemonkey environment into a normal web page. The special ondijitclick handler uses an identity check to compare nodes (e.target === dijit._lastKeyDownNode), but within Greasemonkey references to DOM nodes are wrapped objects. The identity check will therefore fail, even though the nodes are actually the same.

Is there a specific reason for using identity rather than equality? Switching to equality resolves the issue.

Change History (8)

comment:1 Changed 10 years ago by bill

Hi Mark. Hmm, I guess it would work to switch it to an equality check.

Is there a specific reason for using identity rather than equality?

I think the reasoning was:

  1. === seems natural since we are making sure that the event occurred on that exact node, not a node that's "equivalent" to that node
  2. not sure what == means for two DOM nodes, does it return true if they have the same list of attributes and same innerHTML?
  3. performance

comment:2 Changed 10 years ago by Mark Wubben

  1. Agreed. Unfortunately some situations aren't entirely natural
  2. Logically, if everything is taken into account the parentNode and siblings should also be compared, so there can't be an equal copy
  3. I wonder if === really helps with performance. Haven't done any tests but I'd assume that there is an internal identifier for DOM objects that is used when comparing them, whether through == or ===.

comment:3 Changed 10 years ago by bill

Milestone: tbd1.5

comment:4 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:5 Changed 9 years ago by bill

Owner: set to bill
Status: newassigned

OK, I tested the performance and behavior and == seems fine. I ran:

var s1 = dojo.create("span", {innerHTML: "hello world"}),
s2 = dojo.create("span", {innerHTML: "hello world"});
console.log("same: ", s1 === s2);
console.log("equal: ", s1 == s2);

var start = new Date();
for(var i=0, j=0; i<100000; i++)
    if(s1 == s1) j++;
console.log("double equals time: ", new Date() - start);

start = new Date();
for(i=0, j=0; i<100000; i++)
    if(s1 === s1) j++;
console.log("triple equals time: ", new Date() - start);

On IE6 it's:

double equals time: 15 triple equals time: 16

On FF3.6 it's much slower, but still either one is fine:

same: false equal: false double equals time: 128 triple equals time: 131

comment:6 Changed 9 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [22617]) Use == rather than === to make greasemonkey happy, fixes #10250 !strict.

comment:7 Changed 8 years ago by bill

Resolution: fixed
Status: closedreopened

Unfortunately the above fix breaks IE, I filed #12696 to track it. So I'm going to roll it back. Hopefully the greasemonkey (or firefox?) problem has been fixed with FF4 or the newer version of greasemonkey. It's hard to test since I also found another problem, #12698.

comment:8 Changed 8 years ago by bill

Resolution: fixed
Status: reopenedclosed

Hmm, I retested and changing the == back to === doesn't solve the IE9 problem for. I'm just going to fix the regression through #12698.

Note: See TracTickets for help on using tickets.