Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#12476 closed enhancement (fixed)

make html5 fastpath option parse standard attributes directly on DOMNode, outside of data-dojo-props

Reported by: jchase Owned by: bill
Priority: high Milestone: 1.7
Component: Parser Version: 1.6.0
Keywords: fastpath, html5, dojo-data-props Cc: John Hann
Blocked By: Blocking:

Description (last modified by bill)

Refs #12470 and the infamous comment:ticket:11490:60,

This is obviously late in the HTML5 discussion but I have to voice that I completely agree with @unscriptable's post on #11490. I realize the decision has already been made for an "all-or-nothing" use of html5 valid data-dojo-type & data-dojo-props in 1.6 but I hope things can be worked out for 2.0 because as it stands right now, the "fastpath" option is completely unusable to me. It makes sense to me that native attributes e.g.




name="formElementName" (ESPECIALLY!!)

alt="alternate text"

title="additional info"

should be parsed into a dijit always, since those are the native HTML attributes that make up the base HTML node I am trying to "dijify". Having to repeat attributes violates the DRY principle, but in my case specifically as a CakePHP developer it makes it more difficult to wrap html nodes (especially input elements) in dijits now since many of these attributes (name & value, especially) are set via the use of a Form helper which knows HTML. I'm sure I can't be the only one that would have a real problem with this.

I don't agree with the confusion that might be caused by the example provided by @bill:

<button data-dojo-type="dijit.form.Button" type=submit iconClass=save>

appearing to only "half-work" because it seems clear that any non-standard HTML attributes shouldn't be parsed because they don't belong in valid HTML5 markup anyways.

I agree that a performance hole exists in trying to parse an unknown number of attributes on a dom node but I think parsing a known number of native attributes is acceptable and expected. I also agree with certain instances where we don't want a native attribute to be parsed into a dijit (e.g. ContentPane title attribute).

The whole benefit behind the dijit templating system for me has always been that I can write an html node and dijify it (which for me means extending it) if I want to - without having to do anything "weird" (e.g specify attributes twice). This way if I have a framework that is "html-aware", it's perfect - because I don't have to modify the framework I'm using to be "dojo-aware".

I am just hoping this is being addressed for 2.0.

Attachments (2)

nativeParamParse.patch (25.7 KB) - added by bill 11 years ago.
possible patch, refactoring parser to use node.attributes or Jared's cloneNode() trick rather than playing battleship
nativeParamsParse.patch (54.1 KB) - added by bill 11 years ago.
newer patch w/lots of tests, but still working on it

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 years ago by bill

Cc: John Hann added
Component: GeneralParser
Description: modified (diff)
Milestone: tbdfuture
Owner: changed from anonymous to bill
Summary: Seems that html5 fastpath option introduced in 1.6 should at the very least parse the name attribute of input elementsmake html5 fastpath option parse standard attributes directly on DOMNode, outside of data-dojo-props
Type: defectenhancement

Yes, Pete, you, and unscriptable have expressed this concern, I can see the argument.

I imagine you don't care too much, but what's the exact description of "standard attribute"? For example, say that an app wants to use <div> instead of <input> so that it can use <script> tags, like:

<div data-dojo-type="dijit.form.TextBox" type="password">
    <script type="dojo/method" event="onChange> ... </script>

(IIRC you can't use <input> above since it can't have children.)

Are you saying that:

  1. above example should work
  2. type needs to be in data-dojo-props, since it's not a standard attribute on <div>
  3. html5 syntax shouldn't support the <script> tag to setup handlers, just use data-dojo-props
  4. you also want onChange to be specified as a standard attribute <input data-dojo-type=dijit.form.TextBox onChange="myHandler"/>
  5. don't care

PS: updated the above description to link directly to unscriptable's comment.

comment:2 Changed 11 years ago by jchase

My natural inclination would be to opt for #2 (but I do see your point - there's definitely a distinction between native attributes and the idea of "standard" attributes).

But to clarify, my thinking is: if it were an <input>, I would expect the type attribute to be parsed in since it's native (or, if thought from the perspective of someone who maybe doesn't have a ton of dojo experience, he/she would expect the type attribute to not mysteriously "disappear" - that's more likely how it's perceived). The moment I decide I need to change it to a div though, I would naturally think to move type into data-dojo-props since I know it's no longer a native attribute (and it's not really a true input node I'm describing anymore) and I know it then becomes the only way the parser will find it.

As an aside: I ran the example through the validator @ as HTML5 and interestingly it didn't bark at me for having a div with a type attribute specified. I could be missing something?

comment:3 Changed 11 years ago by bill

I'm not sure about, I've been surprised by that tool too, specifically that it doesn't complain about the embedded <script> tags like in my sample code above.

And I agree, people perceive the type attribute as disappearing, even though as you know that's not what's really happening.

Also, I meant to type "native attributes" in my original description, not "standard".

Anyway, there are quite a few native attributes (see, although unfortunately they aren't all listed in the same place, so it's hard to count, but would need to make a list of all of them and their types (or at least the types of the ones that aren't strings).

comment:4 Changed 11 years ago by jchase

With a plethora of native attributes to account for, I acknowledge this is definitely a problem where I can only see the tip of the iceberg - keeping track of which attributes belong natively to which element is its own maintenance nightmare, and I'm sure one that dojo shouldn't be doing.

When I thought (incorrectly) that you were making a distinction between "native" and "standard", I thought perhaps you were suggesting that there should be a fixed short list of most commonly used attributes (arbitrary, I know), without caring to determine if an attribute is native to an element - e.g. a type attribute on a <div> tag. (And again - arbitrary - but the short list I'm thinking of immediately are type, name, class, style, alt, title).

But in this way, the parser doesn't have the extra overhead of having to validate the markup - after all, it's ultimately up to the developer to write valid html5 or not - but the developer doesn't lose the ability to express things simply (and once) for both html & dojo's sake in the more common instances.

comment:5 Changed 11 years ago by bill

Agreed, it's simpler not to worry about which attributes apply to which tags.

The main problem is that on IE6 & IE7 it's hard to find out which attributes were specified for a given tag. node.attributes lists all 80 attributes for a node, not just the attributes specified in the markup. (On IE8+ and other browsers, node.attributes just lists the attributes the user specified.) Besides the performance problem (which is less of an issue now given the decline in usage of IE6/7), there's a functionality issue about type and value (see comment:ticket:11490:64 and comment:ticket:11490:65).

Jared has a trick in some Editor plugin where he makes a shallow copy of a node and then accesses that copy's markup searching for foo= pattern matches. Maybe I should try that for IE6/7. Note to self, here's a test case from IE6:

> var node ="<button id=button4 foo=bar>hello<span>world</span></button", dojo.body());
> clone = node.cloneNode(false);
> clone.outerHTML
<BUTTON id=button4 foo="bar">

Changed 11 years ago by bill

Attachment: nativeParamParse.patch added

possible patch, refactoring parser to use node.attributes or Jared's cloneNode() trick rather than playing battleship

comment:6 Changed 11 years ago by bill

Milestone: future1.7
Status: newassigned

Changed 11 years ago by bill

Attachment: nativeParamsParse.patch added

newer patch w/lots of tests, but still working on it

comment:7 Changed 11 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [24185]) Refactor parser to allow attributes (for a single node) to be partly specified in data-dojo-props, and partly specified directly ex: value=123. Uses node.attributes to detect which attributes are specified on a node, or for older versions of IE calls cloneNode(false) followed by some regex's on clone.outerHTML.

Due to lowercase/uppercase issues (ex: tabIndex, onClick), and for type conversion, the code still introspects each widget to get it's attribute metadata. In the future, would like to defer/avoid that in the common case. Fixes #10153, #12423, #12476, #10150, #9823, refs #11490 !strict.

Also fixes the problem on IE8 where a button without type=... defaults to type=submit rather than whatever the widget defines the default as, fixes #10163, refs #9334, #8946.

Future updates will be attached to ticket #12476.

comment:8 Changed 11 years ago by bill

(In [24355]) fix failure in NodeList.html test on IE, when parser called on HTML fragment not in DOM tree, refs #12476 !strict

comment:9 Changed 11 years ago by bill

(In [25598]) Changes about automatically copying widget attributes to the DOMNode: better detection for which widget attributes are valid DOMNode attributes, to handle:

  • attribute names with dashes (ex: accept-charset)
  • attribute names where the DOMNode JS object's corresponding attribute is mixed case (ex: noValidate)

Since it determines which widget attributes correspond to DOMNode attributes by looking at the DOMNode JS object, it doesn't handle HTML5 attributes like novalidate for browsers that don't understand those attributes.

Note that checking the DOMNode JS object for an attribute name is complicated since the parser returns all attribute names in lowercase (since node.attributes reports them that way), even when the markup uses mixed case, ex: "noValidate".

Also, note that dojo.attr(node, "novalidate", true) doesn't work on IE6 because it's setting a boolean value where the attribute name has incorrect capitalization. So therefore new MyWidget({novalidate: true}) or even MyWidget({noValidate: true}) won't work correctly. However, new MyWidget({noValidate: ""}) will work, and that's what the parser does.

Refs #12476, fixes #13237 !strict.

Note: See TracTickets for help on using tickets.