Opened 5 years ago

Closed 5 years ago

#11372 closed Bug (fixed)

Special characters are encoded twice in nested editables

Reported by: vAx Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.3.2
Component: UI : Widgets Version:
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

Using åäö and alike in the caption gets converted to ä but so does the & part so if you take a look at the source it looks lite this: ä

to reproduce this just go to http://ckeditor.com/demo#widgets and the image tool heading, change the "Apollo CMS-LM spacecraft" caption to include ä for example then take a look at the source.

TC

  1. Open image2 sample.
  2. Paste "åäö" into the caption.
  3. Switch between modes twice.
  4. In wysiwyg you get "åäö".

Change History (10)

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

Component: GeneralUI : Widgets
Description: modified (diff)
Keywords: image2 removed
Milestone: CKEditor 4.3.2
Status: newconfirmed
Summary: using åäö in the caption, enhanced image pluginSpecial characters in caption are encoded twice in nested editables
Version: 4.3.1

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

Summary: Special characters in caption are encoded twice in nested editablesSpecial characters are encoded twice in nested editables

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

I believe that I wrote tests against doubled encoding. So this is not a widget's only issue - most likely it's a conflict with the entities plugin.

comment:4 Changed 5 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

comment:5 Changed 5 years ago by Olek Nowodziński

Status: assignedreview

Pushed solution to t/11372 (+test branch).

It turned out to be much more complex than expected:

  1. The problem was that, because of applyToAll used in entities plugin to solve #11250, dataFilter rule was executed twice: for all editor contents and for nested editable. Double execution implied double replacement:
    return text.replace( baseEntitiesRegex, getChar ).replace( entitiesRegex, getEntity );
    
  1. To avoid re-writing dataFilter logic from scratch (it's a matter of time though), I introduced excludeNestedEditable option so that filtering and replacement are done only once, for nested editable only, while filtering of editor contents excludes nested editables.
  1. Surprisingly, I realized that despite of excludeNestedEditable the problem still occured. It was because filtering defined in Entities plugin was executed in between two toDataFormat listeners of different priorities (8 and 13) in Widget plugin. Those listeners, designed to downcast but to preserve data-cke-* attributes for the "target filtering", precisely the first one, downcasted nested editables, which made excludeNestedEditable unaware of the "history" of parsed element and incapable of protecting its contents.

I postponed downcasting of nested editables and put it into the second toDataFormat listener, which seems to be harmless (at least tests pass).

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

Status: reviewreview_failed

A rule:

addRules( {...}, { excludeNestedEditable: true } )

will be applied to all elements in:

<div><p contenteditable="true">x</p></div>

That's because isRuleApplicable makes assumption that context.nestedEditable may be true only if context.nonEditable is true. They should be independent to avoid weird situations like the one above.

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

Owner: changed from Olek Nowodziński to Piotrek Koszuliński
Status: review_failedassigned

DUP closed #11391.

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

Status: assignedreview

Pushed t/11372 branches. They contain additional tests and fix for issue mentioned in comment:6.

comment:9 Changed 5 years ago by Olek Nowodziński

Status: reviewreview_passed

comment:10 Changed 5 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

git:d138ae0 landed in master (18501ac tests).

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