Opened 12 years ago
Closed 7 years ago
#5456 closed enhancement (patchwelcome)
HTML comments in templates are not removed when template strings are interned.
Reported by: | ptwobrussell | Owned by: | Rawld Gill |
---|---|---|---|
Priority: | low | Milestone: | 1.9 |
Component: | BuildTools | Version: | 1.0 |
Keywords: | needsreview | Cc: | ptwobrussell@… |
Blocked By: | Blocking: |
Description (last modified by )
I noticed that HTML comments aren't removed when template strings are interned. HTML comments can be handy when you have a complex layout with layout dijits and you are trying to keep track of what is what...or if you have to put a copyright blurb in the template during development per a company policy. Seems like stripping these comments for a build would make sense.
Not sure who would be the best person to assign this to, so I thought I'd start somewhere.
Change History (12)
comment:1 Changed 12 years ago by
Owner: | changed from dante to James Burke |
---|
comment:2 Changed 12 years ago by
The HTML comments are never being removed and they appear in my final compressed .js file that's been through shrink safe and the whole 9 yards.
comment:3 Changed 12 years ago by
Milestone: | → 1.1 |
---|
I thought there was an existing bug for this issue (it is a build system issue), but I cannot seem to find it. I'll see about getting into 1.1, but no guarantees yet.
comment:4 Changed 12 years ago by
I would argue that ideally this would be a flag also, as there are cases where inserting comment nodes might be desirable.
comment:5 Changed 12 years ago by
dylan: so you want an option to keep comments in the inlined string (the value set for templateString)? Do you see the comment nodes useful just in development or after a build that is used for "production" use?
Or is more of an issue where removing the comment nodes affects the number of children in a node, and if some code is assuming certain child positions, the code could break? If that is the case, then I am more reticent to do this fix, or perhaps getting the comment stripping behavior is an opt-in option instead of a default that you have to opt out.
comment:6 Changed 12 years ago by
Two possible production use cases. One is like you said, number of nodes. The other is the use case of using a comment node as a place to store data until needed.
comment:7 Changed 12 years ago by
For the first case, I think it's possible, but we should discourage references by index. dojoAttachPoint makes for much less fragile code in general. By that argument, I'd try to make the case that stripping comments should be an opt-out thing, but I don't feel too strongly about it as long as it's an option.
At the risk of over-engineering, if contents in the comments really mattered, we could have a convention for a special comment syntax, e.g. !--*, that is always included and calls attention to the content?
comment:8 Changed 12 years ago by
Milestone: | 1.1 → 1.2 |
---|
comment:9 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Milestone: | 1.2 → future |
comment:10 Changed 8 years ago by
Keywords: | needsreview added |
---|
comment:11 Changed 8 years ago by
Owner: | changed from James Burke to Rawld Gill |
---|---|
Status: | new → assigned |
Personally I would just mark as wontfix, but leaving up to you.
comment:12 Changed 7 years ago by
Milestone: | future → 1.9 |
---|---|
Resolution: | → patchwelcome |
Status: | assigned → closed |
Given the age of this ticket, a patch would be welcome, otherwise this will never be fixed.
Is the bug that HTML comments aren't ever being removed, or only in a particular case?