Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#9205 closed Bug (fixed)

HTMLDataProcessor attribute protection in protectSource method

Reported by: Kyle Florence Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.3.4
Component: General Version: 3.5.2
Keywords: HasPatch Cc:

Description

The regular expression used for attribute matching is overly aggressive (it matches outside of HTML tags). This was resulting in some of the protection characters making their way to our final output (such as: "{C}" or "{cke_protected_1}").

Attachments (2)

9205.patch (624 bytes) - added by Jakub Ś 7 years ago.
9205.html (1.2 KB) - added by Piotrek Koszuliński 5 years ago.
TCs for #9205, #7805 and #8216.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by Jakub Ś

Status: newconfirmed
Version: 3.6.5 (SVN - trunk)3.5.2

This patch seems to fix "{C}" or "{cke_protected_1}". What is more important this patch seems fixes other problems with "{C}", "{cke_protected_1}": #7805, #8216, #7826.

@kflorence could you give us your HTML examples that you had problems with?

Changed 7 years ago by Jakub Ś

Attachment: 9205.patch added

comment:2 Changed 7 years ago by Kyle Florence

See attached. It was especially apparent in French, since there are a lot of words concatenated with a single quote.

comment:3 Changed 7 years ago by Jakub Ś

Thank you for the sample.

comment:4 Changed 7 years ago by Frederico Caldeira Knabben

Reduced TC:

  1. Paste this on source:
<p>A'B<!-- Comment -->C'D</p>
  1. Switch to wysiwyg: BUG: {cke_protected_1} will appear in the comment position.
  1. Switch back to source: BUG: {C} will be appended before the comment.

comment:5 Changed 6 years ago by Chris Smith

Version 3.6.4: The change is needed in the ckeditor.js file. I found the regex and changed it in this file and the problem was fixed.

comment:6 Changed 5 years ago by Eric Pascarello

Is this bug going to be addressed? I am seeing this in the 4.x branch as well.

comment:7 Changed 5 years ago by Wiktor Walc

cc

comment:8 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.3.4
Owner: set to Piotrek Koszuliński
Status: confirmedreview

Pushed branch:t/9205 on dev and tests. I guess that it's still not 100% correct, because it's regexp, but I couldn't find a TC that'd break this.

The patch fixes also #7805 and #8216.

Changed 5 years ago by Piotrek Koszuliński

Attachment: 9205.html added

TCs for #9205, #7805 and #8216.

comment:9 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

comment:10 Changed 5 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:5aba2ea on dev and 87f72be on tests. Patch fixed also #7805 and #8216.

comment:11 Changed 5 years ago by Piotrek Koszuliński

DUP reported in #11672.

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