Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#4385 closed Bug (fixed)

CheckDirty is not returning the correct state

Reported by: Senthil Owned by: Garry Yao
Priority: Must have (possibly next milestone) Milestone: CKEditor 3.1
Component: General Version:
Keywords: Oracle Confirmed Review+ Cc: Pranav, Senthil

Description (last modified by Frederico Caldeira Knabben)

When i set empty data to the editor using the setdata method, checkdirty is returing the wrong state. Please follow the below steps to see the issue.

  1. Open the attached "replacebycode.html" file in IE and click the checkdirty button, this will return the message as 'false'.
  2. Now refresh the page and focus the editor area using the mouse cursor and select the checkdirty button, now you will get a message as "true" eventhough we don't change anything inside the editor area.

Attachments (3)

replacebycode.html (3.3 KB) - added by Senthil 16 years ago.
4385.patch (931 bytes) - added by Garry Yao 16 years ago.
4385_2.patch (1.3 KB) - added by Garry Yao 16 years ago.

Download all attachments as: .zip

Change History (12)

Changed 16 years ago by Senthil

Attachment: replacebycode.html added

comment:1 Changed 16 years ago by Senthil

Priority: NormalHigh

comment:2 Changed 16 years ago by Frederico Caldeira Knabben

Cc: Pranav Senthil added; Pranav Senthil removed
Description: modified (diff)
Milestone: CKEditor 3.xCKEditor 3.1

Changed 16 years ago by Garry Yao

Attachment: 4385.patch added

comment:3 Changed 16 years ago by Garry Yao

Keywords: Confirmed Review? added
Owner: set to Garry Yao
Status: newassigned

Ticket Test added at : http://ckeditor.t/tt/4385/1.html.

comment:4 Changed 16 years ago by Garry Yao

This's one opposite effect of our auto fixing functionality, where the editor's fixing on DOM structure were confusing the dirty checking mechanism, considering it's changes from the user, which is wrong, I've set up a defensive guard for any future auto fixing rules.

comment:5 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The idea about restoring the dirty status is good. I'm only seeing a drawback in the proposed implementation: performance.

The fact is that the checkDirty call is a bit expensive, specially for long documents. Another fact is that the onSelectionChangeFixBody call is made several times during the document life cycle, even when simply typing inside the editor. These facts together may represent some execution overload.

The onSelectionChangeFixBody function will not always perform document fixes. Actually this should happen rarely. So the implementation could be changed to restore the dirty status only if fixes are supposed to happen to the document. It means that checkDirty should be called only if necessary.

comment:6 Changed 16 years ago by Garry Yao

Keywords: Review? added; Review- removed

Changed 16 years ago by Garry Yao

Attachment: 4385_2.patch added

comment:7 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Smart approach ;)

comment:8 Changed 16 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4278].

comment:9 Changed 16 years ago by Garry Yao

Oops, wrong patch used :(
Post-fixing with [4279].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy