Opened 10 years ago
Closed 10 years ago
#11544 closed Bug (fixed)
Placeholders should not be upcasted in parents not accepting spans
| Reported by: | Marcus Bointon | Owned by: | Marek Lewandowski |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.3.4 |
| Component: | UI : Widgets | Version: | 4.3 |
| Keywords: | Cc: |
Description (last modified by )
New description
- Download and open 11544.html.
- Switch between modes.
- See that there's a mess in
<title>tag.
Important note: While working on this issue other scenarios like placeholder used in <textarea> or <video> should be checked. Perhaps more parent tags should be excluded. The rule may be that placeholder may only be created in if, according to DTD, text node's parent accepts span.
Original description (incorrect)
This is related to #11223 and this forum post
I'm using the placeholder plugin in CKEditor 4.3.2, which generally works apart from two issues:
- It doesn't allow it to be used in the root of the head tag (so for example you can't use placeholders to generate meta tags); placeholder get moved into the body tag.
- Placeholder items in the title tag (and possibly elsewhere) get escaped incorrectly, and then fail to be unescaped on save, or when switching to/from source mode.
For example, starting by pasting this document into the source view:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> <html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> <title>[[mytag1]]</title> [[mytag2]] </head> <body>[[mytag3]]</body> </html>
Switching to HTML view then back to source results in:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> <html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> <title><span tabindex="-1" contenteditable="false" data-cke-widget-wrapper="1" data-cke-filter="off" class="cke_widget_wrapper cke_widget_new cke_widget_inline" data-cke-display-name="placeholder"><span class="cke_placeholder" data-cke-widget-keep-attr="0" data-widget="placeholder">[[mytag1]]</span></span></title> </head> <body>[[mytag2]][[mytag3]]</body> </html>
You can see an escaping wrapper has been applied in the title tag, and the [[mytag]] element in the head tag has been moved into the body.
Doing the same switch again results in:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> <html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> <title><span tabindex="-1" contenteditable="false" data-cke-widget-wrapper="1" data-cke-filter="off" class="cke_widget_wrapper cke_widget_new cke_widget_inline" data-cke-display-name="placeholder"><span class="cke_placeholder" data-cke-widget-keep-attr="0" data-widget="placeholder"><span tabindex="-1" contenteditable="false" data-cke-widget-wrapper="1" data-cke-filter="off" class="cke_widget_wrapper cke_widget_new cke_widget_inline" data-cke-display-name="placeholder"><span class="cke_placeholder" data-cke-widget-keep-attr="0" data-widget="placeholder">[[mytag1]]</span></span></span></span></title> </head> <body>[[mytag2]][[mytag3]]</body> </html>
It's now double-escaped, and this repeats each time you switch views or save.
For me this is a complete showstopper; I can't use CKEditor if it causes content explosions.
Attachments (1)
Change History (11)
comment:1 Changed 10 years ago by
| Keywords: | placeholder source removed |
|---|---|
| Milestone: | → CKEditor 4.3.4 |
| Status: | new → confirmed |
| Version: | 4.3.2 → 4.3 |
Changed 10 years ago by
| Attachment: | 11544.html added |
|---|
comment:2 Changed 10 years ago by
That example HTML had a couple of problems, but I turned it into a minimal jsfiddle that reproduces the problem; just toggle the source button.
Note that I disabled protectedSource - placeholder alone triggers the issue.
comment:3 Changed 10 years ago by
| Description: | modified (diff) |
|---|---|
| Summary: | Wrong escaping in title tag, moving tags out of head with placeholders → Placeholders should not be upcasted in parents not accepting spans |
I know that placeholder alone triggers this issue. I wanted to show in 11544.html that if you want to use non-HTML strings in the <head> section you need to use protectedSource. Otherwise those strings will be pulled out to the <body>, which is a correct behaviour.
comment:4 Changed 10 years ago by
| Description: | modified (diff) |
|---|
comment:5 Changed 10 years ago by
Yes, thanks for pointing that out - I will try to think of a way to pre and post-filter my content in <head> to stop it being moved about. Does the jsfiddle work for you?
comment:7 Changed 10 years ago by
| Owner: | set to Marek Lewandowski |
|---|---|
| Status: | confirmed → assigned |
comment:8 Changed 10 years ago by
| Status: | assigned → review |
|---|
Pushed to t/11544 at dev and t/11544 at tests.
comment:9 Changed 10 years ago by
| Status: | review → review_passed |
|---|
Force pushed branches because:
- They were based on major instead of master.
- If non standard editor was used in generic tests its good to initialize it in
async:init, what won't require changes when other setups will be needed. - Fixed the case with custom elements.
comment:10 Changed 10 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with git:8b8b2af2bd (merged to master) at dev and 677dc1a1bc (merged to master) at tests.
Placeholders cannot be used in
<head>. You have to load correct HTML and<head>cannot contain text content. You could solve this withconfig.protectedSourcebut you'll need to use different pattern than for normal placeholders.So the broken scenario is placeholder's format used in the
<title>. See 11544.html. I also used there protectedSource to check if it correctly works in the<head>tag and everything looks fine. What's not fine is a mess in<title>.While working on this issue other scenarios like placeholder used in
<textarea>or<video>should be checked. Perhaps more parent tags should be excluded.