#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)
Change History (7)
Changed 11 years ago by
Attachment: | addClassUndefined.patch added |
---|
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:3 Changed 11 years ago by
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 11 years ago by
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 11 years ago by
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 11 years ago by
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.
Allow undefined classStr parameter to addClass and toggleClass, plus a test for such usage