Opened 6 years ago

Closed 6 years ago

#7749 closed Bug (fixed)

execCommand throws an error when data is empty

Reported by: aorduno Owned by: fredck
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 fredck 6 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by j.swiderski

  • Resolution set to invalid
  • Status changed from new to closed

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 fredck

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

comment:3 Changed 6 years ago by fbingha

"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 fbingha

  • Cc freddie@… added

comment:5 Changed 6 years ago by fredck

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 fbingha

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 fredck

comment:7 Changed 6 years ago by fredck

  • Resolution invalid deleted
  • Status changed from closed to reopened

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 fredck

  • Status changed from reopened to pending

comment:9 Changed 6 years ago by fbingha

Yes, that would work nicely.

comment:10 Changed 6 years ago by fredck

  • Owner set to fredck
  • Status changed from pending to review

comment:11 Changed 6 years ago by garry.yao

  • Status changed from review to review_passed

comment:12 Changed 6 years ago by fredck

  • Milestone set to CKEditor 3.6.1
  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [6973].

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