#4351 closed Bug (fixed)
Dashes cannot be used in attribute names
| Reported by: | Niek Kouwenberg | Owned by: | Garry Yao |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.1 |
| Component: | General | Version: | 3.0 |
| Keywords: | Confirmed HasPatch Review+ | Cc: |
Description
The CKEditor htmlParser uses a regular expression to check for valid attributes. This regex however, does not allow dashes to be used in the attribute name.
If I'm correct, dashes are allowed as character in an attribute name, and the "\w" set, does not include the dash. Therefore, the dash should be added separately (like the colon).
htmlparser.js:
- 21. var attribsRegex = /([\w:]+)... + 21. var attribsRegex = /([\w:\-]+)...
Attachments (2)
Change History (15)
comment:1 Changed 15 years ago by
| Keywords: | Confirmed HasPatch added |
|---|---|
| Milestone: | CKEditor 3.x → CKEditor 3.1 |
comment:2 Changed 15 years ago by
| Owner: | set to Garry Yao |
|---|---|
| Status: | new → assigned |
Changed 15 years ago by
| Attachment: | 4351.patch added |
|---|
comment:3 Changed 15 years ago by
| Keywords: | Review? added |
|---|
comment:4 Changed 15 years ago by
Design TC updated at : http://ckeditor.t/dt/core/htmlparser/htmlparser.html
comment:5 Changed 15 years ago by
| Keywords: | Review- added; Review? removed |
|---|
Looking at the W3C specs, we can see that attribute names accept a complex range of chars. But, let's keep it simple here, stickying with the common cases. So, let's just add "-" and "." to it ("_" is already inside \w).
Changed 15 years ago by
| Attachment: | 4351_2.patch added |
|---|
comment:6 Changed 15 years ago by
| Keywords: | Review? added; Review- removed |
|---|
comment:7 follow-up: 8 Changed 15 years ago by
Please note: In "4351_2.patch", the "." (period) needs to be escaped, it now matches any character!
comment:8 Changed 15 years ago by
| Keywords: | Review+ added; Review? removed |
|---|
Replying to niek:
Please note: In "4351_2.patch", the "." (period) needs to be escaped, it now matches any character!
Yes! Be sure to have it escaped when committing.
comment:9 follow-up: 10 Changed 15 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Fixed with [4349].
comment:10 follow-up: 11 Changed 15 years ago by
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
comment:11 Changed 15 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
Replying to niek:
You still haven't escaped the dot.
After some internal talk and more research, we confirmed that the dot doesn't need to be escaped inside a character class.
comment:12 follow-up: 13 Changed 15 years ago by
Indeed, you are right! My apologies.
Small suggestion; add a comment to related tickets when discussion these things internally. This way you'll prevent confusion and possible new bug reports (since your last comment stated 'be sure to have it escaped', and the diff doesn't show the escape).
comment:13 Changed 15 years ago by
Replying to niek:
Small suggestion; add a comment to related tickets when discussion these things internally. This way you'll prevent confusion and possible new bug reports (since your last comment stated 'be sure to have it escaped', and the diff doesn't show the escape).
You're right. The committer should have added a small comment about it. Sorry for this small mis-observation, and thanks for the suggestion ;)
You are right, one example of such attribute would be http-equiv.
Thanks for report and the fix.