Opened 6 years ago

Closed 6 years ago

#7749 closed Bug (fixed)

execCommand throws an error when data is empty

Reported by: alan Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.6.1
Component: General Version:
Keywords: Cc: freddie@…

Description

i think it would be a good idea to check (if is empty) data before call execCommand.

For example in _source/ywsiwygarea/plugin.js in line 55-56

if ( this.dataProcessor )

data = this.dataProcessor.toHtml( data );

there might be some JS errors since we use our own dataprocessor which can result in data being empty. So for avoiding this i think it would be nice to check if data is empty or something like that before calling execCommand.

Alan Orduno

Attachments (1)

7749.patch (500 bytes) - added by Frederico Caldeira Knabben 6 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by Jakub Ś

Resolution: invalid
Status: newclosed

The default version of htmlDataProcessor always returns at least empty string for data so that it never is empty.

Perhaps a better option would be to modify your dataprocessor to always return something in data.

comment:2 Changed 6 years ago by Frederico Caldeira Knabben

Basically, toHtml() is *required* to always return a HTML string, even if empty.

comment:3 Changed 6 years ago by Freddie Bingham

"empty string for data so that it never is empty"

I am confused. An empty string is not empty? I am not talking null values here but a valid empty string.

Our dataprocessor always returns a string, albeit an empty string sometimes. Why do you say that an empty string is acceptable when execCommand will not accept an empty string on IE?

comment:4 Changed 6 years ago by Freddie Bingham

Cc: freddie@… added

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

I'm also very confused here... you're talking about execCommand, but then you pointed us to dataProcessor.toHtml... can you please be more precise about what execCommand call you're talking about?

comment:6 Changed 6 years ago by Freddie Bingham

function doInsertHtml( data )

The execCommand around line 106 in wysiwygarea/plugin.php .. where the data that is set at the beginning of this function ultimately ends up. This gets executed for NON IE and it throws an error when data is an empty string.

Changed 6 years ago by Frederico Caldeira Knabben

Attachment: 7749.patch added

comment:7 Changed 6 years ago by Frederico Caldeira Knabben

Resolution: invalid
Status: closedreopened

Ah, ok... now it's much clearer.

Is the attached patch the kind of fix you're talking about?

comment:8 Changed 6 years ago by Frederico Caldeira Knabben

Status: reopenedpending

comment:9 Changed 6 years ago by Freddie Bingham

Yes, that would work nicely.

comment:10 Changed 6 years ago by Frederico Caldeira Knabben

Owner: set to Frederico Caldeira Knabben
Status: pendingreview

comment:11 Changed 6 years ago by Garry Yao

Status: reviewreview_passed

comment:12 Changed 6 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.1
Resolution: fixed
Status: review_passedclosed

Fixed with [6973].

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