Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#4305 closed defect (duplicate)

dojo.html.getClass failing in FF when passed SVG node...(old bug?)

Reported by: guest Owned by: sjmiles
Priority: high Milestone:
Component: HTML Version: 0.4.2
Keywords: Cc:
Blocked By: Blocking:

Description

There is an original thread here that very accurately describes what I ran into:

http://trac.dojotoolkit.org/ticket/3878

I did my first ANT build this past friday, so I am relatively new to DOJO, but it looks like a lot of stuff has been moved around so maybe this was re-introduced during a re-write/port.

At any rate, what I am seeing is that dojo.html.getClass accesses node.className which for IE seems to be a string but for FF seems to be an object (e.g. the type is SVGAnimatedString in my case).

Thus the return statement cs.replace(/s+|s+$/g,"") in dojo.html.getClass() fails since this SVG type doesn't support the replace method.

The initial IF condition of dojo.html.getClass() to test for the existence of node.className passes, since its the SVGAnimatedString type and isnt null. It seems that this if was intended to handle HTMLElements first and then pass the buck if it was something funky like the SVG node. Perhaps a slight tweak to this IF logic should allow HTMLElements to hit the first block and other nodes to hit the second block.

For the record, I added a "&& false" to the first IF condition and it worked great for my SVG node, so this seems to be a minor issue.

Best of luck!

Stuart Stephens

Change History (6)

comment:1 Changed 12 years ago by Eugene Lazutkin

Milestone: 0.4.4
Version: 0.90.4.2

comment:2 Changed 12 years ago by sjmiles

As documented in the ticket you referenced (http://trac.dojotoolkit.org/ticket/3878) a patch was committed for this problem.

Are you saying that the the patched version is still incorrect?

comment:3 Changed 12 years ago by guest

I was mistaken regarding the dojo version (the issue I found was in .4).

I refered to that ticket because the details were essentially identical, however I am not sure if this is another permutation or if perhaps some code was shuffled around or what happened. There also seemed to be some discussion about what should/shouldn't be done in this situation, which is another reason I linked.

That said, I did trace into the code and narrowed the error down to the dojo.html.getClass() function (/src/html/style.js line 18) when passing the function an SVG node (get an error in FF only).

Here is the code I saw:

dojo.html.getClass = function(/*HTMLElement*/ node){

summary: Returns the string value of the list of CSS classes currently assigned directly to the node in question. Returns an empty string if no class attribute is found; node = dojo.byId(node); if(!node){ return ""; } var cs = ""; if(node.className){

cs = node.className;

}else if(dojo.html.hasAttribute(node, "class")){

cs = dojo.html.getAttribute(node, "class");

} return cs.replace(/s+|s+$/g, ""); string

}

Input is a SVG node - NOT an HTMLElement, which is key to the error.

What seems to happen is that ...

if(node.className)

... on line 13 evaluates true for an SVG node input, as node.className evaluates to an SVG object, NOT a string. This will bomb out on line 18 when a string operation is attempted since the var cs is NOT a string.

I think the fix from the refered bug resulted in the code here ...

else if(dojo.html.hasAttribute(node, "class")){

cs = dojo.html.getAttribute(node, "class");

}

... as this code works beautifully. The trick is the initial condition is always true for the SVG node input and the second else/if is never executed as a result.

Does that help?

Regards,

Stuart

comment:4 Changed 12 years ago by sjmiles

Resolution: duplicate
Status: newclosed

There is no confusion as to the nature of the problem, as you mentioned it was already documented in #3878.

So, this is already fixed in the 0.4 source branch, see this changeset as noted in the referenced ticket http://trac.dojotoolkit.org/changeset/10067.

comment:5 Changed 12 years ago by guest

My mistake, I thought your initial post refered to the old ticket. I should have read closer.

Stuart

comment:6 Changed 12 years ago by (none)

Milestone: 0.4.4

Milestone 0.4.4 deleted

Note: See TracTickets for help on using tickets.