Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11146 closed defect (fixed)

[REGRESSION] addClass and toggleClass can't handle an undefined classStr parameter in 1.4

Reported by: Revin Guillen Owned by: Revin Guillen
Priority: high Milestone: 1.5
Component: Core Version: 1.4.3
Keywords: Cc:
Blocked By: Blocking:

Description

r19075 seems to have broken the handling for dojo.addClass(node, some_undefined_variable) and toggleClass(node). In 1.3, these run without error, but in 1.4, it throws an error. Calling these functions this way is an obvious problem, but it's an easy thing for the toolkit to catch.

The smallest patch I can see is to return s || ""; in the s2array() function.

Attachments (1)

addClassUndefined.patch (1006 bytes) - added by Revin Guillen 9 years ago.
Allow undefined classStr parameter to addClass and toggleClass, plus a test for such usage

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by Revin Guillen

Attachment: addClassUndefined.patch added

Allow undefined classStr parameter to addClass and toggleClass, plus a test for such usage

comment:1 Changed 9 years ago by Revin Guillen

(In [22168]) Fix a regression in which addClass and toggleClass can no longer handle and undefined classStr parameter. (refs #11146) !strict

comment:2 Changed 9 years ago by Revin Guillen

Resolution: fixed
Status: newclosed

comment:3 Changed 9 years ago by James Burke

So why is it good to preserve the behavior in 1.3? It seems to me getting an error earlier on a bad call is better, so it is easier to correct?

comment:4 Changed 9 years ago by Revin Guillen

I can see that, sure. My thinking was simply for backwards compatibility reasons, but if it's not the kind of thing that needs to be backwards compatible, it's easy enough to monkey patch addClass at the application level, and we can note this in the docs instead.

comment:5 Changed 9 years ago by Eugene Lazutkin

I am with James on this one: instead of masking subtle bugs we should alert programmers as early as possible.

In general I am -0 on fixing this issue. Seeing it being fixed already I am not advocating a rollback, unless there are some strong objections to the fix from other developers.

comment:6 Changed 9 years ago by James Burke

Agreed with elazutkin, I would not have "fixed" this (error out early is preferred), but I do not feel strongly enough about it to roll back the applied fix since the change does not break anything. In the future though, I suggest giving a bit more consideration before making changes in behavior for dojo base.

Note: See TracTickets for help on using tickets.