Opened 11 years ago

Closed 11 years ago

#6665 closed defect (fixed)

Using htmlFor vs. for as an attribute getter/setter

Reported by: jgarfield Owned by: Becky Gibson
Priority: high Milestone: 1.2
Component: Dijit Version: 1.1.0
Keywords: Cc:
Blocked By: Blocking:

Description

I noticed when working with the latest Editor code in Internet Explorer, the labels for 'Font Size' and 'Font Name' (when using the FontChoice?.js Plugin) don't have their tags closed, nor do they have ids associated with them.

Further investigating this, the line that was the culprit in this Plugin was the following in the setToolbar function:

label.setAttribute("for", forRef.id);

I found that in Internet Explorer (you may want to test more browsers than those that I covered), you must use 'htmlFor' instead of 'for' for the setAttribute/getAttribute call. Same goes for class vs. className.

Currently dojo.attr() only looks at tabIndex vs. tabindex for Internet Explorer. I think we should also add another case to _fixAttrName for 'for' -> 'htmlFor' and 'class' -> 'className'. The Plugin in question should also probably use dojo.attr() as well to make use of its corrections.

This also affects dojo.query!

dojo.query([for="blah"]);

versus

dojo.query([htmlFor="blah"]);

I tested the following browsers to see which ones would/wouldn't accept for and/or htmlFor, and these are the results...

IE8 - Uses htmlFor
IE7 - Uses htmlFor
IE6 - Uses htmlFor

FF3 (PC) - Uses for
FF2 (PC) - Uses for
FF1.5 (PC) - Uses for
Opera9 (PC) - Uses for
Mozilla 1.8 (PC) - Uses for
Safari 3.0 (PC) - Uses for
Safari 3.1 (PC) - Uses for
Netscape 7.2 (PC) - Uses for
Netscape 8.1 (PC) - Uses for

Change History (11)

comment:1 Changed 11 years ago by bill

Ouch.... so you are saying that neither "for" nor "htmlFor" works in all browsers. I guess we need to normalize for this in dojo.attr() and dojo.query() (unless it just calls dojo.attr()), and then change lingering node.setAttribute() calls to use dojo.attr().

I don't really understand the part about "not having their tags closed" but maybe that's just a red herring that led you to this issue?

comment:2 Changed 11 years ago by jgarfield

Ah, sorry for not explaining "not having their tags closed" very well.

When I would inspect the LABEL elements in the IE Developer Toolbar, they would look like the following...

<LABEL for=

There was no closing angle-bracket or anything...Could just be a bug with the Developer Toolbar though (one of the many, lol)

comment:3 Changed 11 years ago by Adam Peller

Component: HTMLDijit
Owner: changed from sjmiles to Becky Gibson

comment:4 Changed 11 years ago by bill

Please note this really isn't a dijit bug. It's mainly about dojo.attr() and dojo.query() (which need some automated tests for "for", btw, please add them). It also touches dijit.Editor and dojox.dtl.

comment:5 Changed 11 years ago by Becky Gibson

actually htmlFor is defined in the DOM and all of the following support labelObject.htmlFor

  • IE6
  • IE 7
  • FF 2.0.0.14 Mac & Win
  • FF3 beta Win (haven't tested Mac)
  • Opera 9.5
  • Safari 3.1.1 Mac & Win

IE supports labelObject.getAttribute("htmlFor") and FF, Opera, Safari support labelObject.getAttribute("for"); None of the browsers I tested supported labelObject.for.

comment:6 Changed 11 years ago by Becky Gibson

I'm not sure that the problem reported for the editor is actually a problem. My tests show that IE does support setAttribute("for") and I do not see any issues with the font choices label elements in the IE developer toolbar. Can you provide any actual error condition?

There are issues on how to get the for/htmlFor attribute and the method used depends upon the browser and how the for attribute was initially set. I do agree that dojo.attr needs to be updated but that can probably wait until 1.2.

comment:7 Changed 11 years ago by Becky Gibson

Milestone: 1.1.11.2
severity: majornormal

comment:8 Changed 11 years ago by Becky Gibson

(In [13820]) refs #6665 Updated _fixAttrName and hasAttr to deal with idiosyncracies of htmlFor and for attributes. Created tests. !strict

comment:9 Changed 11 years ago by Becky Gibson

Status: newassigned

comment:10 Changed 11 years ago by bill

See also #6957 (a duplicate of this bug?). Perhaps this is fixed too.

comment:11 Changed 11 years ago by Becky Gibson

Resolution: fixed
Status: assignedclosed

closing - not sure why I marked as refs and not fixed when I made the checkin.

Note: See TracTickets for help on using tickets.