Opened 7 years ago

Closed 7 years ago

#9995 closed Bug (fixed)

should not change html inside textarea

Reported by: yiminghe Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.1.1
Component: Core : Parser Version: 3.0
Keywords: HasPatch Cc:

Description

steps:

  1. open editor and switch to source mode, input the following code:

<textarea><img src='xx.png' /></textarea>

  1. switch to wysiwyg mode and then switch to source mode again

expected:

editor code is

<p><textarea>&lt;img src=&#39;xx.png&#39; /&gt;</textarea></p>

actual:

editor code is

<p><textarea>&lt;img data-cke-saved-src=&#39;xx.png&#39; src=&#39;xx.png&#39; /&gt;</textarea></p>

PS:

I know it is better to write escaped html inside textarea, but it is easier for user to write raw html inside textarea and browser does not complain either.

PS2:

In htmldataprocessor it is better to add textarea into protectElementsRegex and put protectElements on the top of the toHtml function, but then ckeditor's htmlparser will fail to parse <textarea><img /></textare>, so i think it needs a change to the htmlparser of ckeditor: allow any content inside tag textarea except </textarea>

Change History (9)

comment:1 Changed 7 years ago by yiminghe

ckeditor moved to github finally :), here is my pull request:

https://github.com/ckeditor/ckeditor-dev/pull/25

comment:2 Changed 7 years ago by Jakub Ś

Currently the only way to insert HTML into textarea is using textarea dialog. It creates desired results.

@yiminghe one thing for sure attributes like cke_saved should not be created.

comment:3 Changed 7 years ago by Jakub Ś

Resolution: duplicate
Status: newclosed

This is a duplicate of #5294.

@yiminghe I have pasted your pull request in that ticket. Taking however #5294 into account I'm not sure if approach is valid.

comment:4 Changed 7 years ago by Piotrek Koszuliński

Keywords: HasPatch added
Resolution: duplicate
Status: closedreopened

I'm reopening this issue, because it is not a dup of #5294. That ticket is about parsing, this about processing.

Your patch seems to be correct. Content of textarea indeed should be protected before we protect other things.

And you're not right about parsing: <textarea><img /></textare> - not that you made a typo :). After correcting it everything works fine.

We'll work on this ticket for the next minor release. Meanwhile, I pushed rebased t/9995 on cksource remote.

comment:5 Changed 7 years ago by Jakub Ś

Status: reopenedconfirmed
Version: 4.0.23.0

Problem can be reproduced from CKEditor 3.0 in both CKEditor 3.x and 4.x

comment:6 Changed 7 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.1.1

I'm adding it to 4.1.1 milestone conditionally - if the patch is good (or doesn't need a lot of additional work), let's use it. If not, then we won't be able work on it for 4.1.1.

comment:7 Changed 7 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedreview

Pushed t/9995b on dev and tests.

I fixed this original issue with textarea but also too greedy <style> protection.

comment:8 Changed 7 years ago by Olek Nowodziński

Status: reviewreview_passed

The very last test test removing apple style span with the very last assertion made me laugh ;) Since Reinmar claims this was really missing and somehow I still find most of his decisions credible, spring R+ has landed.

comment:9 Changed 7 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:0f90f84 on dev and 5f38bbc on tests.

Regarding tests... I think the we didn't understand each other, because I don't know what's funny about that one :D

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy