Opened 12 years ago

Closed 11 years ago

#5226 closed defect (fixed)

dijit._Templated does not replace booleans correctly

Reported by: guest Owned by: Adam Peller
Priority: high Milestone: 1.2
Component: Dijit Version: 1.0
Keywords: Cc: ptwobrussell@…
Blocked By: Blocking:

Description (last modified by ptwobrussell)

If a widget contains e.g:

myBool: false

and your template contains:

<div dojoType="myWidget" value="${myBool}"></div>

value is evaluated to true.

This happened after changeset r10979 when the following was added on line 64 in the buildRendering function:

if(!value){ return ""; }  

The buildRendering function now replaces ${myBool} with "" instead of "false".

After this the str2obj function is called in the dojo parser. In the boolean case the following code is run:

return typeof value == "boolean" ? value : !(value.toLowerCase()=="false"); 

Since value is "" it returns true.

Regards, Thomas

Attachments (1)

foo.html (1.2 KB) - added by ptwobrussell 11 years ago.
A test case illustrating the behavior - useful to illustrate the bug.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 12 years ago by Adam Peller

Owner: set to alex

comment:2 Changed 12 years ago by bill

Is this problem specific to the parameter named "value" (which is special, btw), or does it happen with any parameter name?

comment:3 Changed 12 years ago by guest

It happens with any parameter name.

comment:4 Changed 12 years ago by Adam Peller

Milestone: 1.0.21.0.3

comment:5 Changed 12 years ago by alex

Status: newassigned

comment:6 Changed 12 years ago by dylan

Milestone: 1.0.31.1

comment:7 Changed 11 years ago by bill

Priority: highestnormal

comment:8 Changed 11 years ago by bill

Milestone: 1.11.2

Move all milestone 1.1 tickets to 1.2, except for reopened tickets and tickets opened after 1.1RC1 was released.

Changed 11 years ago by ptwobrussell

Attachment: foo.html added

A test case illustrating the behavior - useful to illustrate the bug.

comment:9 Changed 11 years ago by ptwobrussell

Cc: ptwobrussell@… added
Description: modified (diff)

comment:10 Changed 11 years ago by bill

Milestone: 1.21.4
Owner: changed from alex to bill
Status: assignednew

comment:11 Changed 11 years ago by ptwobrussell

Maybe this is the patch? Seems to make sense and works on the test case provided.

Index: _Templated.js
===================================================================
--- _Templated.js	(revision 14945)
+++ _Templated.js	(working copy)
@@ -43,7 +43,7 @@
 			return dojo.string.substitute(tmpl, this, function(value, key){
 				if(key.charAt(0) == '!'){ value = _this[key.substr(1)]; }
 				if(typeof value == "undefined"){ throw new Error(className+" template:"+key); } // a debugging aide
-				if(!value){ return ""; }
+				if(value === ""){ return ""; }
 
 				// Substitution keys beginning with ! will skip the transform step,
 				// in case a user wishes to insert unescaped markup, e.g. ${!foo}

comment:12 Changed 11 years ago by bill

I think the issue is that when value isn't a string (like when it's a boolean) then the .charAt() on the next line fails, at least on some browsers.

comment:13 Changed 11 years ago by bill

Oops, I meant toString(), not charAt(). My point is that your patch reverts the behavior to as it was before [10979], but presumably there's some reason Alex changed that.

comment:14 Changed 11 years ago by Adam Peller

only thing I think needs to be safeguarded is a null check. perhaps that's what was intended. would it be more appropriate to return "" or throw as with undefined? need to test against the test case in #4643 on IE, I guess.

comment:15 Changed 11 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [15166]) Fixes #5226, regression of boolean attribute handling in _Templated. Thanks, ptwobrussell! Refs #4643

comment:16 Changed 11 years ago by Adam Peller

Milestone: 1.41.2

comment:17 Changed 11 years ago by bill

Resolution: fixed
Status: closedreopened

The test case added above fails on IE. It assume DOM node attributes appear in a certain order (<span num="-5" value="true">) but IE prints them in the opposite order.

Also another error after that I didn't track down.

comment:18 Changed 11 years ago by bill

Owner: changed from bill to Adam Peller
Status: reopenednew

comment:19 Changed 11 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [15328]) Fix tests on IE. Fixes #5226. Tag attribute order is unspecified.

Note: See TracTickets for help on using tickets.