Ticket #4385 (closed Bug: fixed)

Opened 5 years ago

Last modified 5 years ago

CheckDirty is not returning the correct state

Reported by: Senthil Owned by: garry.yao
Priority: High Milestone: CKEditor 3.1
Component: General Version:
Keywords: Oracle Confirmed Review+ Cc: Pranav, Senthil

Description (last modified by fredck) (diff)

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

replacebycode.html (3.3 KB) - added by Senthil 5 years ago.
4385.patch (931 bytes) - added by garry.yao 5 years ago.
4385_2.patch (1.3 KB) - added by garry.yao 5 years ago.

Change History

Changed 5 years ago by Senthil

comment:1 Changed 5 years ago by Senthil

  • Priority changed from Normal to High

comment:2 Changed 5 years ago by fredck

  • Cc Pranav, Senthil added; Pranav Senthil removed
  • Description modified (diff)
  • Milestone changed from CKEditor 3.x to CKEditor 3.1

Changed 5 years ago by garry.yao

comment:3 Changed 5 years ago by garry.yao

  • Keywords Confirmed Review? added
  • Owner set to garry.yao
  • Status changed from new to assigned

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

comment:4 Changed 5 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 5 years ago by fredck

  • 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 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

Changed 5 years ago by garry.yao

comment:7 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

Smart approach ;)

comment:8 Changed 5 years ago by garry.yao

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed with [4278].

comment:9 Changed 5 years ago by garry.yao

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

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