#8492 closed enhancement (fixed)
Whitespace in tempates does the wrong thing?
Reported by: | ryangroth5 | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.3 |
Component: | Dijit | Version: | 1.2.3 |
Keywords: | Cc: | [email protected]…, Eugene Lazutkin | |
Blocked By: | Blocking: |
Description
In a strange svn merge that left me with newlines in my template files I observed that the chain of execution from dijit._Templated threw errors with trailing whitespace in widget templates.
// summary: // Construct the UI for this widget from a template, setting this.domNode. // Lookup cached version of template, and download to cache if it // isn't there already. Returns either a DomNode or a string, depending on // whether or not the template contains ${foo} replacement parameters. var cached = dijit._Templated.getCachedTemplate(this.templatePath, this.templateString, this._skipNodeCache); var node; if(dojo.isString(cached)){ node = dojo._toDom(this._stringRepl(cached)); }else{ // if it's a node, all we have to do is clone it node = cached.cloneNode(true); } this.domNode = node; // recurse through the node, looking for, and attaching to, our // attachment points and events, which should be defined on the template node. this._attachTemplateNodes(node);
The error,in short, is that _Templated gets the template code (which looks like "<div>...</div>\n" in the error case ) then passes it to dojo._toDom() which converts the block to a DocumentFragment? (because of the hanging \n). This gets passed to _attachTemplateNodes() which expects a dom node NOT a document fragment.
This is an artifact of dojo._toDom creating a document fragment in the case that there is more than one node in the incoming string (our second node is the "\n" text node).
atachTemplateNodes doesn't know what to do with this and throws a method not found error. In the case of no trailing whitespace, then we get our node and everything proceeds.
So, what I would suggest would be one of the following:
1) Throw an error in attachTemplateNodes if you get a document fragment, or indeed, anything you don't want.
2) Trim templates somewhere so that you don't get document fragments when dojo users/extenders are accidentally leaving trailing whitespace in their template files.
3) Document profusely that NO whitespace be allowed in the widget template files
THIS ERROR APPLIES TO 1.3pre. I haven't noticed it before 1.3.
Thanks,
Ryan CLA on file
Change History (10)
comment:1 Changed 13 years ago by
Owner: | set to Eugene Lazutkin |
---|
comment:2 Changed 13 years ago by
Cc: | Eugene Lazutkin added |
---|---|
Owner: | changed from Eugene Lazutkin to bill |
Actually it is the expected behavior of dojo._toDom()
--- it can be used to create text nodes as well. The best way to handle it is to add dojo.trim()
to _Templated
like you suggested.
comment:3 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [16482]) Trim leading/trailing space from template files in case there is a trailing newline, which causes dojo._toDom() to return a DocumentFragment? (the real node and a trailing text node) rather than a single node.
Fixes #8492 !strict.
comment:4 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Same issue, but now I am getting this on the build version. What is happens is that the build tool puts the newline into the template string. The template string bypasses the fix in [16482] since [16482] addresses only the template file. The template file newline issue is fixed, however.
Thanks,
Ryan
comment:5 follow-up: 6 Changed 13 years ago by
OK. I know what is at issue here. Apparently dojo uses the native string trim if it i available (I don't know which browser might do this) in dojo.trim(). Because of namespace polution from another library my browser suddenly defined an inferior string trim operation which doesn't take care of the "\n" case. Sorry for reopening this. I'd consider it closed unless you want to take care of the dojo code assuming that String.prototype.trim is native.
comment:6 Changed 13 years ago by
Replying to ryangroth5:
Because of namespace polution from another library my browser suddenly defined an inferior string trim operation which doesn't take care of the "\n" case.
Which library is doing this?
comment:8 Changed 13 years ago by
Interesting. OpenLayers? uses this replace for its trim:
replace(/^\s*(.*?)\s*$/, "$1")
which generally strips \r and \n though some strange combinations don't work, such as
" \ra\nbc\n ".replace(/^\s*(.*?)\s*$/, "$1")
OpenLayers? overrides String.prototype.trim, but the override is now marked as 'deprecated' so they must have decided it was a mistake to do so... perhaps they can be encouraged to remove it?
comment:9 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Hmm, OK, I'm going to close this ticket again.
You could open another ticket about the Dojo/OpenLayers incompatibility, but I'm skeptical that this is worth working-around in dojo. There is a workaround possible in your code too: you can redefine String.prototype.trim to work correctly.
comment:10 Changed 13 years ago by
I'd encourage you to file a ticket against OpenLayers? as well. They may want to look at implementations here:
http://blog.stevenlevithan.com/archives/faster-trim-javascript
Eugene, so this really seems like an error/unwanted behavior with dojo._toDom(). Can you change that code to do a trim? Or should I add a trim to _Templated?