Opened 7 years ago

Closed 5 years ago

#15503 closed defect (fixed)

NodeList-data contains logic error

Reported by: iCanDo Owned by: Kris Zyp
Priority: low Milestone: 1.10
Component: Core Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description

Executing

query(node).data();

as getter, lang.mixin (for setter) should not be executed.

if(arguments.length == 1){ r = dataCache[pid]; }

should be

if(arguments.length == 1){ return dataCache[pid]; }

Change History (10)

comment:1 Changed 7 years ago by dante

Milestone: tbd1.8
Owner: set to dante
Status: newassigned

comment:2 Changed 7 years ago by dante

good catch. debugging shows current unit tests calling mixin() 80% of the time when it doesn't need to. The call is mostly a no-op because it is being passed undefined most times, but useless in the getter case.

tried adding a test connecting to dojo.mixin and incrementing a counter, but I guess lang.mixin !== dojo.mixin, or there is an ordering difference or something, but couldn't get it to fire. Other tests don't regress when applying change.

comment:3 Changed 7 years ago by iCanDo

My solution has an error, because it not return undefined, if no data is stored.

Better solution:

if( ! dataCache[pid] && arguments.length > 1 ) {
  dataCache[pid] = {};
}

if(arguments.length == 1){ return dataCache[pid]; }

comment:4 Changed 7 years ago by dante

your solution didn't change the behavior in any way. the empty literal is always created, so an empty dataset is always that literal. never undefined. you can have undefined keys, and that will return undefined, but that's not the same situation as "empty getter".

comment:5 in reply to:  4 Changed 7 years ago by iCanDo

Next try:

if( typeof key == "string" ) {
  ...
} else if( arguments.length == 2 ) {
  r = lang.mixin(dataCache[pid], key);
}

comment:6 Changed 7 years ago by Colin Snover

Milestone: 1.82.0

1.8 has been tagged; moving all outstanding tickets to next major release milestone.

comment:7 Changed 6 years ago by dylan

Milestone: 2.01.9
Owner: changed from dante to kriszyp
Priority: undecidedlow

comment:8 Changed 6 years ago by bill

Owner: changed from kriszyp to Kris Zyp

comment:9 Changed 6 years ago by bill

Milestone: 1.91.10

Bumping this ticket since we are past the deadline for the 1.9RC. The fix can be put into 1.9.1 too, if desired.

comment:10 Changed 5 years ago by Kris Zyp <kriszyp@…>

Resolution: fixed
Status: assignedclosed

In 3ad07a0497649e266ba99a9e347eb9b41bd92dd6/dojo:

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.